diff --git a/migrations/0194_orb_relay_pending_enroll_id.sql b/migrations/0194_orb_relay_pending_enroll_id.sql new file mode 100644 index 0000000000..a1afb43fa9 --- /dev/null +++ b/migrations/0194_orb_relay_pending_enroll_id.sql @@ -0,0 +1,11 @@ +-- Pull-mode drain isolation (#9150): orb_relay_pending was scoped only by installation_id, so ANY enrollment +-- secret valid for an installation could drain and destructively-ack another live consumer's queued webhooks +-- (a stale re-enrolled container silently stealing a fresh one's events). This column lets an enqueue tag +-- which enrollment a GitHub-webhook row was queued for (the same "winning enrollment" forwardOrbEvent already +-- picks for the push path, #1783); NULL for every existing row and for config_push notices (which remain +-- intentionally installation-wide, not consumer-specific -- see enqueueConfigPushRelay). NULL also means "not +-- yet tagged", so pullRelayPending's WHERE clause matches untagged rows regardless of which enrollment asks -- +-- a safe default during rollout, not a bypass (an enrollment can only ever add matches, never lose ones scoped +-- to a DIFFERENT enroll_id). +ALTER TABLE orb_relay_pending ADD COLUMN enroll_id TEXT; +CREATE INDEX IF NOT EXISTS idx_orb_relay_pending_enroll ON orb_relay_pending (installation_id, enroll_id); diff --git a/packages/loopover-engine/src/telemetry/anonymize.ts b/packages/loopover-engine/src/telemetry/anonymize.ts index 78786186b8..315958fc0e 100644 --- a/packages/loopover-engine/src/telemetry/anonymize.ts +++ b/packages/loopover-engine/src/telemetry/anonymize.ts @@ -14,6 +14,12 @@ import { createHmac, randomBytes } from "node:crypto"; * once per instance in its own store and reuses it on every export, so the same raw value always hashes the * same way. Never derived from, or shared with, any other credential (App private keys, webhook secrets) -- * key separation means a leaked anonymization secret can't be used to forge or decrypt anything else. + * + * #9147: this now also holds for Orb's opt-in federated peer-benchmark export (src/orb/federated-bundle.ts). + * That feature signs its bundles with its OWN dedicated secret (getOrCreateFederatedSigningSecret) rather + * than this one, and derives its instance identity from a third, independent secret + * (getOrCreateInstanceIdentitySecret) -- so enrolling a federated peer, or rotating any one of the three + * secrets, can never affect this anonymization secret or the always-on export that uses it. */ export function generateAnonSecret(): string { return randomBytes(32).toString("hex"); diff --git a/src/api/routes.ts b/src/api/routes.ts index 98c5a862f6..65547d40f3 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -166,6 +166,7 @@ import { isOrbBrokerEnabled, issueOrbEnrollment, issueOrbStoredSecret, + ORB_SECRET_TYPE_GITHUB_TOKEN, ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL, revokeOrbEnrollment, } from "../orb/broker"; @@ -1761,14 +1762,15 @@ export function createApp() { }); // Federated benchmark (#6481): "your gate precision vs peer median". Reads the opt-in from the loopover // self-repo's manifest (mirrors prReconciliation/publicStats/etc.'s fleet-wide override lookup) rather - // than any of the maintainer's own repos — federatedIntelligence is operator-level, not per-repo. Bounded - // to a single, short-timeout attempt so an unreachable or slow collector degrades the panel to its - // existing empty state instead of holding up the whole dashboard load. + // than any of the maintainer's own repos — federatedIntelligence is operator-level, not per-repo. + // #9148: buildFederatedBenchmark no longer pulls a peer collector on this request path at all — the + // local half is a single fast DB query, and the peer half is read from a cache the "federated-peer-sync" + // background queue job refreshes on its own cadence. This used to be a live, rate-limited network call + // on every dashboard load (N maintainers hitting refresh was N requests/second at the peer collector); + // now a slow/unreachable collector can only make the CACHE stale, never hold up this request. const federatedIntelligenceManifest = await loadRepoFocusManifest(c.env, resolveLoopOverSelfRepoFullName(c.env)); const federatedBenchmark = await buildFederatedBenchmark(federatedIntelligenceManifest, c.env.DB, { now: Date.parse(generatedAt), - timeoutMs: 5_000, - maxAttempts: 1, }); const qualityDashboard = { ...buildMaintainerQualityDashboard({ @@ -4441,16 +4443,19 @@ export function createApp() { } catch { ack = undefined; // tolerate an empty/invalid body — just no ack this round } - const events = await pullRelayPending(c.env, enrollment.installationId, { ack }).catch(dbBrokerError); + // #9150: scope the drain to THIS enrollment (or untagged/legacy rows) — not merely the installation — so a + // second live enrollment for the same install can never steal and destructively-ack this one's events. + const events = await pullRelayPending(c.env, enrollment.installationId, { ack, enrollId: enrollment.enrollId }).catch(dbBrokerError); if (events === null) return c.json({ error: "broker_error" }, 503); return c.json({ events }, 200); }); // LoopOver Orb (#1255) — central fleet-calibration collector. Receives anonymized, reversal-aware outcome // batches from self-hosted instances. Sender-side HMAC anonymization is for privacy, not authentication. - // OPTIONAL shared-token gate (#1285): unset ⇒ OPEN ingress (the live fleet keeps working, as before); set - // ⇒ the collector REQUIRES it, so an operator can lock the write path down after distributing the matching - // ORB_COLLECTOR_TOKEN to exporters. Bounded by a hard body ceiling, and dedup'd via UNIQUE(instance_id, repo_hash, pr_hash). + // #9046/#9166: FAILS CLOSED when ORB_INGEST_TOKEN is unset — isAuthorizedIngest (below, shared with + // /v1/ams/ingest) requires an exact bearer match against the configured token; an unconfigured collector + // rejects rather than accepting anonymous writes. Bounded by a hard body ceiling, and dedup'd via + // UNIQUE(instance_id, repo_hash, pr_hash). app.post("/v1/orb/ingest", async (c) => { if (!(await isAuthorizedIngest(c.env.ORB_INGEST_TOKEN, extractBearerToken(c.req.header("authorization"))))) return c.json({ error: "unauthorized" }, 401); const body = await readOrbIngestBody(c.req.raw, c.req.header("content-length")); @@ -4575,15 +4580,22 @@ export function createApp() { // records lands at registered=0; only REGISTERED ones count toward the global public counter (getOrbGlobalStats) // and are eligible for token brokering. Bearer-gated by the `/v1/internal/*` middleware (INTERNAL_JOB_TOKEN). The // list shows pending + registered installs so an operator knows what they're opting in. + // + // liveEnrollmentCount (#9149): how many enrollment secrets are currently live ('enrolled', not revoked) for + // this install — an operator previously had no way to see that a re-enrollment (issueOrbEnrollment was a bare + // INSERT, never revoking) had accumulated a second standing credential. A correlated subquery rather than a + // JOIN: each installation has at most a handful of enrollment rows, so this stays cheap without reshaping the + // one-row-per-installation result set a GROUP BY would require. app.get("/v1/internal/orb/installations", async (c) => { const rows = await c.env.DB .prepare( `SELECT installation_id AS installationId, account_login AS accountLogin, account_type AS accountType, repository_selection AS repositorySelection, registered, suspended_at AS suspendedAt, - removed_at AS removedAt, first_seen_at AS firstSeenAt, last_event_at AS lastEventAt + removed_at AS removedAt, first_seen_at AS firstSeenAt, last_event_at AS lastEventAt, + (SELECT COUNT(*) FROM orb_enrollments oe WHERE oe.installation_id = orb_github_installations.installation_id AND oe.state = 'enrolled' AND oe.revoked_at IS NULL) AS liveEnrollmentCount FROM orb_github_installations ORDER BY last_event_at DESC`, ) - .all<{ installationId: number; accountLogin: string | null; accountType: string | null; repositorySelection: string | null; registered: number; suspendedAt: string | null; removedAt: string | null; firstSeenAt: string; lastEventAt: string }>(); + .all<{ installationId: number; accountLogin: string | null; accountType: string | null; repositorySelection: string | null; registered: number; suspendedAt: string | null; removedAt: string | null; firstSeenAt: string; lastEventAt: string; liveEnrollmentCount: number }>(); return c.json({ installations: (rows.results ?? []).map((r) => ({ ...r, registered: r.registered === 1 })) }); }); @@ -4619,9 +4631,15 @@ export function createApp() { // secret issuance path control-plane's hosted provisioning core (#7180/#8066) calls instead, for a credential // that already exists (a tenant's Postgres connection string) rather than a GitHub installation to bind. // `installationId` is irrelevant to that path (see issueOrbStoredSecret's own header comment for why). + // + // Also accepts an optional `{ rotate: true }` (#9149): when set, every PRIOR live enrollment for this + // installation is revoked before the new one is minted, mirroring the OAuth landing page's `state=:rotate` + // flow (oauth.ts) for the operator-issued path. Defaults to false (append, not replace) -- unchanged behavior + // for every existing caller, since a blue/green container swap legitimately relies on two live enrollments + // briefly overlapping (see #9150's sibling fix, which makes that overlap safe rather than assuming away). app.post("/v1/internal/orb/enrollments", async (c) => { if (!isOrbBrokerEnabled(c.env)) return c.json({ error: "not_found" }, 404); - const payload = (await c.req.json().catch(() => null)) as { installationId?: unknown; secretType?: unknown; secretValue?: unknown } | null; + const payload = (await c.req.json().catch(() => null)) as { installationId?: unknown; secretType?: unknown; secretValue?: unknown; rotate?: unknown } | null; if (payload?.secretType === ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL) { const secretValue = typeof payload.secretValue === "string" ? payload.secretValue : ""; const result = await issueOrbStoredSecret(c.env, ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL, secretValue); @@ -4630,7 +4648,7 @@ export function createApp() { } const installationId = Number(payload?.installationId); if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "installationId required" }, 400); - const result = await issueOrbEnrollment(c.env, installationId); + const result = await issueOrbEnrollment(c.env, installationId, undefined, ORB_SECRET_TYPE_GITHUB_TOKEN, { rotate: payload?.rotate === true }); if ("error" in result) return c.json(result, result.error === "installation_not_found" ? 404 : 409); return c.json(result); // { enrollId, secret } — secret shown exactly once }); @@ -6690,10 +6708,6 @@ function toIsoQueryDate(value: string): string | undefined { } -// Optional Orb-ingest auth (#1285). FAIL-OPEN by default: with no ORB_INGEST_TOKEN configured the ingress stays -// OPEN (matching today's live fleet — deploying this is non-breaking). Once the operator sets the token, the -// collector REQUIRES an exact bearer match, so the write path can be locked down after the matching -// ORB_COLLECTOR_TOKEN is rolled out to exporters. /** * #9046: the SINGLE authorization rule for every telemetry-collector ingest endpoint (Orb and AMS), so the two * products cannot drift apart again. They previously had separate, near-identical copies — and AMS ended up diff --git a/src/db/schema.ts b/src/db/schema.ts index 805e29dfd0..b75ce4fc3d 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -977,6 +977,11 @@ export const orbRelayPending = sqliteTable( // enqueueConfigPushRelay). The dispatch side (#7523) branches on this before treating raw_body as a // GitHubWebhookPayload. kind: text("kind").notNull().default("github_webhook"), + // Pull-mode drain isolation (#9150): which enrollment a GitHub-webhook row was queued for (the same + // "winning enrollment" forwardOrbEvent picks for the push path). NULL for config_push rows (installation- + // wide, not consumer-specific) and for any pre-#9150 row -- pullRelayPending's WHERE matches NULL + // regardless of the asking enrollment, so an untagged row is never silently orphaned. + enrollId: text("enroll_id"), }, (table) => ({ installation: index("idx_orb_relay_pending_install").on(table.installationId, table.createdAt), @@ -986,6 +991,8 @@ export const orbRelayPending = sqliteTable( // pruneRelayPending (src/orb/relay.ts) filters/deletes by created_at alone (fleet-wide TTL sweep, not // scoped to one installation) -- neither index above leads with created_at, so that scan was unindexed. createdAt: index("idx_orb_relay_pending_created_at").on(table.createdAt), + // #9150: pullRelayPending's SELECT/DELETE now filter on (installation_id, enroll_id) together. + enroll: index("idx_orb_relay_pending_enroll").on(table.installationId, table.enrollId), }), ); diff --git a/src/index.ts b/src/index.ts index 1ad18b400f..486ce9ff2e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ import { isLoopEscalationSweepEnabled } from "./review/loop-escalation-wire"; import { isAprRepoTransferPollEnabled } from "./orb/apr-repo-transfer"; import { isPrReconciliationEnabled, resolvePrReconciliationManifestOverride } from "./review/pr-reconciliation"; import { isActiveReviewReconciliationEnabled, resolveActiveReviewReconciliationManifestOverride } from "./review/active-review-reconciliation"; +import { resolveFederatedIntelligenceManifestOverride } from "./orb/federated-benchmark"; import { isRagEnabled } from "./review/rag-wire"; import { isDecisionAuditEnabled } from "./review/decision-audit"; import { isRiskControlEnabled } from "./review/risk-control-wire"; @@ -226,6 +227,16 @@ async function enqueueScheduledJobs(env: Env, controller: ScheduledController): jobs.push({ type: "reconcile-active-review-tracking", requestedBy: "schedule" }); } } + // Federated peer-sync tick (#9148/#9166). Same 10-minute cadence as the reconciliation jobs above — a + // best-effort background sync has no business running any more often than that, and the collector's own + // module-level rate limiter (federated-collector.ts, 6/min) further bounds any single tick's calls. Opt-in + // via the loopover self-repo's `.loopover.yml federatedIntelligence:` block only — there is no env-var + // equivalent for this one (unlike prReconciliation/activeReviewReconciliation above), since federated intel + // has never had one; flag-OFF (default, `present: false`) this job is never created. + if (isReconciliationWindow) { + const federatedIntelligenceManifestOverride = await resolveFederatedIntelligenceManifestOverride(env); + if (federatedIntelligenceManifestOverride.enabled) jobs.push({ type: "federated-peer-sync", requestedBy: "schedule" }); + } if (isHourly) { // Isolation (#experimental-gittensor-plugin): on self-host, refresh-registry both FETCHES from and // PERSISTS the whole upstream gittensor-subnet registry (entrius/gittensor has no server-side filtering, diff --git a/src/orb/broker.ts b/src/orb/broker.ts index fa77468c4d..874a2f9c96 100644 --- a/src/orb/broker.ts +++ b/src/orb/broker.ts @@ -53,16 +53,26 @@ export type IssueResult = { enrollId: string; secret: string } | { error: "insta * hashed). Issued by the operator (internal endpoint) OR by a maintainer who proved install-admin via OAuth — * in the latter case the maintainer's GitHub identity is recorded for audit. installation_id is bound here and * read back (never from the request) at token-exchange time, so a secret can never mint a token for another - * install. `secretType` defaults to the only mintable type today; every existing caller is unaffected. */ + * install. `secretType` defaults to the only mintable type today; every existing caller is unaffected. + * + * `opts.rotate` (#9149): this was previously a bare INSERT, so re-running the enrollment flow (e.g. after a + * secret leak) minted a SECOND simultaneously-valid secret and revoked nothing — the leaked one kept working + * forever. Deliberately NOT the default: a blue/green container swap or a deliberate multi-container setup + * legitimately relies on two live enrollments overlapping for a short window (see #9150's sibling fix, which + * makes that overlap safe rather than assuming it can't happen) — auto-revoking on every issuance would break + * that. `rotate: true` is the caller's explicit "I want this to be the only valid secret now" confirmation + * (the OAuth landing page's `?rotate=1`/`state=:rotate` flow, oauth.ts). */ export async function issueOrbEnrollment( env: Env, installationId: number, maintainer?: { login: string; githubId?: number | null | undefined }, secretType: string = ORB_SECRET_TYPE_GITHUB_TOKEN, + opts: { rotate?: boolean } = {}, ): Promise { const install = await env.DB.prepare("SELECT registered FROM orb_github_installations WHERE installation_id = ?").bind(installationId).first<{ registered: number }>(); if (!install) return { error: "installation_not_found" }; if (install.registered !== 1) return { error: "installation_not_registered" }; + if (opts.rotate === true) await revokeAllLiveEnrollmentsForInstallation(env, installationId); const enrollId = createOpaqueToken("orbenr"); const secret = createOpaqueToken("orbsec"); await env.DB.prepare( @@ -74,6 +84,28 @@ export async function issueOrbEnrollment( return { enrollId, secret }; } +/** Revokes every currently-live ('enrolled', not yet revoked) enrollment for an installation, regardless of + * secret_type — the self-hoster-reachable counterpart to the single-enrollment {@link revokeOrbEnrollment} + * (#9149). Returns the count actually revoked (0 is a valid, non-error result: an install with no live + * enrollment is not a failure, it's just already in the state the caller wanted). Deliberately does NOT gate + * on `registered`/`suspended_at`/`removed_at` the way issuance does — revoking access should work EVEN ON an + * installation the operator has since disabled or suspended, since a maintainer revoking a leaked secret has + * no reason to be blocked by an unrelated administrative state. */ +export async function revokeAllLiveEnrollmentsForInstallation(env: Env, installationId: number): Promise { + const result = await env.DB.prepare("UPDATE orb_enrollments SET revoked_at = CURRENT_TIMESTAMP, state = 'revoked' WHERE installation_id = ? AND state = 'enrolled' AND revoked_at IS NULL") + .bind(installationId) + .run(); + return result.meta.changes; +} + +/** How many enrollments are currently live ('enrolled', not revoked) for an installation (#9149) — the count + * surfaced to an operator (the internal installations list) and to a maintainer (the OAuth secret page) so + * neither has to guess whether a re-enrollment minted an extra standing credential. */ +export async function countLiveEnrollmentsForInstallation(env: Env, installationId: number): Promise { + const row = await env.DB.prepare("SELECT COUNT(*) AS c FROM orb_enrollments WHERE installation_id = ? AND state = 'enrolled' AND revoked_at IS NULL").bind(installationId).first<{ c: number }>(); + return row?.c ?? 0; +} + export type IssueStoredSecretResult = IssueResult | { error: "secret_value_required" | "encryption_unavailable" }; /** Issues a one-time enrollment secret for a STORED (not minted) credential (#8064) -- e.g. control-plane's diff --git a/src/orb/federated-benchmark.ts b/src/orb/federated-benchmark.ts index c5d7db0629..7f59402eec 100644 --- a/src/orb/federated-benchmark.ts +++ b/src/orb/federated-benchmark.ts @@ -1,46 +1,102 @@ // LoopOver federated fleet intelligence (#1970) — the dashboard benchmark (#6481): "your gate precision vs -// peer median". Composes the three already-shipped pipeline stages — export (#6478, federated-bundle.ts), -// transport (#6479, federated-collector.ts), and trust-gated import (#6480, federated-import.ts) — into the -// one comparison the maintainer dashboard renders. No new storage, no new network primitive: this module is -// pure composition over functions that already exist and already fail safe on their own. +// peer median". Composes the already-shipped pipeline stages — export (#6478, federated-bundle.ts), transport +// (#6479, federated-collector.ts), and trust-gated import (#6480/#9148, federated-import.ts) — into the one +// comparison the maintainer dashboard renders. +// +// #9148 SPLIT THIS MODULE IN TWO, on a request-time/background-tick line: +// - {@link buildFederatedBenchmark} is the REQUEST-TIME read path (called from the dashboard route). It +// computes the LOCAL half live (a single local DB query, cheap) but reads the PEER half from a cache this +// module writes on a background tick — it never pulls a peer collector itself. Before #9148, this +// function made a live network call (with a rate limiter, a 5s timeout, and a 1-MB response ceiling) on +// EVERY maintainer dashboard load; N maintainers hitting refresh was N requests/second at the peer +// collector, and every load paid that request's full latency. +// - {@link refreshFederatedBenchmarkCache} is the BACKGROUND-TICK write path (called from the federated +// peer-sync queue job, src/queue/job-dispatch.ts's "federated-peer-sync" case). It does the actual pull + +// trust-gate + persisted-watermark pipeline and writes the result to a `system_flags` cache row. This is +// the only place in this module that touches the network. // // FAIL-SAFE BY COMPOSITION, not by a wrapping try/catch: buildFederatedBundle degrades to null on any error, -// pullPeerBundles degrades to [] on any error or when not opted in, and importPeerBundles is a pure function -// with no I/O. None of the three can throw, so this module doesn't need to catch anything either — wrapping -// it in another try/catch would only hide which stage actually failed. -import { buildFederatedBundle, isFederatedIntelligenceEnabled } from "./federated-bundle"; -import { importPeerBundles } from "./federated-import"; -import { pullPeerBundles, type CollectorOpts } from "./federated-collector"; +// pullPeerBundles degrades to [] on any error or when not opted in, and importPeerBundles/ +// applyFederatedPeerWatermarks degrade their own way on failure (a DB read failure reads as "cache empty" / +// "every peer state unknown", never a thrown error). Neither function here needs its own catch as a result. +import { buildFederatedBundle, isFederatedIntelligenceEnabled, resolveFederatedWindowDays } from "./federated-bundle"; +import { applyFederatedPeerWatermarks, importPeerBundles } from "./federated-import"; +import { pullPeerBundles, pushFederatedBundle, type CollectorOpts } from "./federated-collector"; import { percentile } from "./analytics"; +import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; +import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest"; import type { FocusManifest } from "../signals/focus-manifest"; export interface FederatedBenchmark { /** This instance's own P(merged & not reverted | gate said merge), from buildFederatedBundle. Null below - * MIN_DECIDED, exactly like the exported bundle field it reuses. */ + * MIN_DECIDED, exactly like the exported bundle field it reuses. Computed LIVE at request time — this is + * a single local DB query, not a network call, so there is no reason to cache it. */ localMergePrecision: number | null; - /** Median mergePrecision across every accepted (trust-gated) peer bundle that itself cleared MIN_DECIDED. - * Null when no peer contributed a numeric value yet — an empty-state condition, not an error. */ + /** Median mergePrecision across every accepted (trust-gated, range-checked, fresh, per-instance-deduped, + * Sybil-capped — #9148) peer bundle that itself cleared MIN_DECIDED, as of the last background sync tick. + * Null when no peer has ever contributed one — an empty-state condition, not an error. */ peerMedianMergePrecision: number | null; - /** How many peers actually contributed to the median above (i.e. passed trust-gating AND had a non-null - * mergePrecision) — NOT the raw count of bundles pulled or accepted, which may include peers still below - * their own MIN_DECIDED threshold. */ + /** How many DISTINCT peer instances contributed to the median above, as of the last background sync tick. */ peerCount: number; generatedAt: string; } +/** system_flags key for the cached peer-half of the benchmark (#9148) — written only by + * {@link refreshFederatedBenchmarkCache}, read only by {@link buildFederatedBenchmark}. */ +const FEDERATED_BENCHMARK_CACHE_FLAG_KEY = "orb:federated_benchmark_cache"; + +interface FederatedBenchmarkCache { + peerMedianMergePrecision: number | null; + peerCount: number; + /** When the cache was last refreshed — surfaced so a stale-cache condition (sync tick disabled/broken) is + * at least visible in the raw system_flags row, even though the dashboard field above always reports its + * OWN generatedAt (the read time, not the cache's write time). */ + refreshedAt: string; +} + +/** Fails safe to null on any error (missing row, corrupt JSON, a D1 outage) — a cache miss reads exactly like + * "opted in, no peer data yet", the same empty state #6481 already treats as normal rather than an error. */ +async function readFederatedBenchmarkCache(db: D1Database): Promise { + try { + const row = await db.prepare("SELECT value FROM system_flags WHERE key = ?").bind(FEDERATED_BENCHMARK_CACHE_FLAG_KEY).first<{ value: string }>(); + if (!row?.value) return null; + const parsed: unknown = JSON.parse(row.value); + if (parsed === null || typeof parsed !== "object") return null; + const cache = parsed as Partial; + if (typeof cache.peerCount !== "number" || typeof cache.refreshedAt !== "string") return null; + return { + peerMedianMergePrecision: typeof cache.peerMedianMergePrecision === "number" ? cache.peerMedianMergePrecision : null, + peerCount: cache.peerCount, + refreshedAt: cache.refreshedAt, + }; + } catch { + return null; + } +} + +/** Best-effort write. A write failure must never fail the sync tick — the next tick simply overwrites the + * still-stale cache again. */ +async function writeFederatedBenchmarkCache(db: D1Database, cache: FederatedBenchmarkCache): Promise { + await db + .prepare("INSERT OR REPLACE INTO system_flags (key, value, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP)") + .bind(FEDERATED_BENCHMARK_CACHE_FLAG_KEY, JSON.stringify(cache)) + .run() + .catch(() => undefined); +} + /** - * Build the local-vs-peer-median benchmark for the maintainer dashboard. + * Build the local-vs-peer-median benchmark for the maintainer dashboard — the REQUEST-TIME read path (#9148). * * Returns null — touching nothing beyond the opt-in check — when federated intelligence is not enabled for - * this deployment (`federatedIntelligence.enabled` off in the loopover self-repo's manifest). This is the - * "an instance that hasn't opted in sees no new UI, not an empty/disabled version of it" gate #6481 requires; - * the caller renders no panel at all on a null result, distinct from a real object with peerCount: 0 (opted - * in, no peer data yet — an empty state, not an error). + * this deployment. Otherwise ALWAYS computes the local half live (a single, fast, local DB query) and reads + * the peer half from the background-tick cache; it never pulls a peer collector itself. A cache miss (no + * sync tick has run yet, e.g. right after opting in) reads as peerCount 0 / a null median — the SAME + * empty-state shape a genuinely peerless deployment already renders, not an error and not a missing panel. */ export async function buildFederatedBenchmark( manifest: Pick | null | undefined, db: D1Database, - opts: { now?: number; windowDays?: number } & CollectorOpts = {}, + opts: { now?: number; windowDays?: number } = {}, ): Promise { if (!isFederatedIntelligenceEnabled(manifest)) return null; @@ -49,21 +105,105 @@ export async function buildFederatedBenchmark( // passed, so an omitted opts.windowDays falls through to buildFederatedBundle's own default instead of // being overridden with an explicit undefined. const localBundle = await buildFederatedBundle(manifest, db, opts.windowDays === undefined ? { now } : { now, windowDays: opts.windowDays }); + const cached = await readFederatedBenchmarkCache(db); + + return { + localMergePrecision: localBundle?.mergePrecision ?? null, + peerMedianMergePrecision: cached?.peerMedianMergePrecision ?? null, + peerCount: cached?.peerCount ?? 0, + generatedAt: new Date(now).toISOString(), + }; +} + +/** + * Pull + trust-gate + persist every peer bundle this instance can reach, then cache the resulting median for + * {@link buildFederatedBenchmark} to read — the BACKGROUND-TICK write path (#9148). Called only from the + * federated peer-sync queue job, never from any request path. + * + * A no-op (cache untouched) when not opted in — an opted-out instance never overwrites a stale cache with a + * fresh "no data" result, so re-enabling later doesn't need to wait out a sync tick to see its last known + * peer state disappear for no reason. Every stage below already fails safe on its own (see this file's header + * comment), so this function needs no wrapping try/catch either. + */ +export async function refreshFederatedBenchmarkCache( + manifest: Pick | null | undefined, + db: D1Database, + opts: { now?: number; windowDays?: number } & CollectorOpts = {}, +): Promise { + if (!isFederatedIntelligenceEnabled(manifest)) return; + + const now = Number.isFinite(opts.now) ? (opts.now as number) : Date.now(); + const localWindowDays = resolveFederatedWindowDays(opts.windowDays); + const config = manifest?.federatedIntelligence; const peerBundles = await pullPeerBundles(manifest, opts); - const { accepted } = importPeerBundles(manifest, peerBundles); + const stateless = importPeerBundles(manifest, peerBundles, { now, localWindowDays }); + const { accepted } = config ? await applyFederatedPeerWatermarks(db, stateless, config.peerKeys, { now }) : stateless; + // MEDIAN, NOT MEAN (mirrors analytics.ts's own fleet aggregation, see federated-import.ts's header comment): // a bounded number of outliers cannot drag a median arbitrarily, so re-deriving a mean here would quietly // weaken the same poisoning-resistance property the import side already relies on holding by construction. + // `accepted` is deduped one-bundle-per-instanceId by importPeerBundles (#9148), so this count is already a + // count of distinct CONTRIBUTING INSTANCES, not raw bundles — the bug the peerCount doc used to warn about. const peerMergePrecisions = accepted .map((bundle) => bundle.mergePrecision) .filter((value): value is number => value !== null) .sort((a, b) => a - b); - return { - localMergePrecision: localBundle?.mergePrecision ?? null, + await writeFederatedBenchmarkCache(db, { peerMedianMergePrecision: percentile(peerMergePrecisions, 50), peerCount: peerMergePrecisions.length, - generatedAt: new Date(now).toISOString(), - }; + refreshedAt: new Date(now).toISOString(), + }); +} + +/** + * The full federated peer-sync tick (#9148/#9166): refreshes the peer-median cache AND pushes this instance's + * own bundle to the operator's configured collector, in parallel. This is the function the "federated-peer- + * sync" queue job calls; it is the concrete wiring #9166 deferred ("pushFederatedBundle wiring into a real + * tick lands with the #9148 commit, which introduces the background refresh job this naturally slots into"). + * + * `Promise.allSettled` rather than `Promise.all`: a push failure must never skip the pull-side refresh (and + * vice versa) — the two directions are independent and each already fails safe to a falsy/empty result on + * its own, so there is nothing here worth rejecting the whole tick over. + */ +export async function runFederatedPeerSyncTick( + manifest: Pick | null | undefined, + db: D1Database, + opts: { now?: number; windowDays?: number } & CollectorOpts = {}, +): Promise { + await Promise.allSettled([refreshFederatedBenchmarkCache(manifest, db, opts), pushFederatedBundle(manifest, db, opts)]); +} + +/** Config-as-code override for the SCHEDULER's own enqueue gate (#9148) — mirrors resolveOpsManifestOverride + * (src/review/ops-wire.ts) exactly: a short in-isolate TTL cache over the loopover self-repo's manifest, so + * the cron tick doesn't pay a full manifest load on every 2-minute pass just to decide whether to enqueue a + * job the processor will re-check anyway (defense in depth — the queue job-dispatch case loads the full + * manifest itself before doing any real work, exactly like ops-alerts does). A load failure degrades to + * `{ enabled: false }`, so a manifest hiccup can only ever under-enqueue, never wrongly arm the sync. */ +export type FederatedIntelligenceManifestOverride = { present: boolean; enabled: boolean }; + +const FEDERATED_MANIFEST_OVERRIDE_CACHE_TTL_MS = 60_000; +let federatedManifestOverrideCache: { override: FederatedIntelligenceManifestOverride; at: number } | null = null; + +export async function resolveFederatedIntelligenceManifestOverride(env: Env, nowMs: number = Date.now()): Promise { + const hit = federatedManifestOverrideCache; + if (hit && nowMs - hit.at < FEDERATED_MANIFEST_OVERRIDE_CACHE_TTL_MS) return hit.override; + try { + const manifest = await loadRepoFocusManifest(env, resolveLoopOverSelfRepoFullName(env)); + const config = manifest.federatedIntelligence; + const override = { present: config.present, enabled: config.enabled === true }; + federatedManifestOverrideCache = { override, at: nowMs }; + return override; + } catch { + const override = { present: false, enabled: false }; + federatedManifestOverrideCache = { override, at: nowMs }; + return override; + } +} + +/** Test-only: clears the cached override, mirroring clearOpsManifestOverrideCacheForTest — without this, a + * test suite running many cases would leak one test's cached override into the next. */ +export function clearFederatedIntelligenceManifestOverrideCacheForTest(): void { + federatedManifestOverrideCache = null; } diff --git a/src/orb/federated-bundle.ts b/src/orb/federated-bundle.ts index 1cfeb36842..304efa38ff 100644 --- a/src/orb/federated-bundle.ts +++ b/src/orb/federated-bundle.ts @@ -3,7 +3,8 @@ // This is the EXPORT side only: it packages a subset of this instance's own local calibration data into a // signed, anonymized bundle an operator can choose to hand to a peer. It performs NO network call — the // transport (push/pull against an operator-configured collector) is #6479, the receiving/trust-gating side is -// #6480, and the key-trust scheme is #6477's design (see the TODO on the signing key below). +// #6480, and the key-trust scheme is #6477's design (see signFederatedBundle's own doc comment below for the +// dedicated signing key #9147 introduced). // // NOT the same thing as the #1255 orb export (src/selfhost/orb-collector.ts:155). That path is deliberately // distinct on five axes, and this module exists precisely because none of them can be retrofitted onto it: @@ -25,8 +26,8 @@ // #6481 renders "this instance's gate precision vs the peer median", so a bundle's mergePrecision must be // computed by the exact same confusion-matrix definition the fleet median uses, or the comparison is // apples-to-oranges. Same reason MIN_DECIDED gates the published precision here. -import { createHmac } from "node:crypto"; -import { bucketReasonCode, cycleTimeMs, getOrCreateAnonSecret, instanceId } from "../selfhost/orb-collector"; +import { createHmac, randomBytes } from "node:crypto"; +import { bucketReasonCode, cycleTimeMs, getOrCreateInstanceIdentitySecret, instanceId } from "../selfhost/orb-collector"; import { foldInstance, MIN_DECIDED, percentile, type Cell as FleetCell } from "./analytics"; import type { FocusManifest } from "../signals/focus-manifest"; @@ -35,9 +36,20 @@ import type { FocusManifest } from "../signals/focus-manifest"; export const FEDERATED_BUNDLE_SCHEMA_VERSION = 1; /** Default calibration window. Mirrors computeFleetAnalytics' 90-day default and 365-day clamp so a bundle's - * window is directly comparable to the fleet's. */ -const DEFAULT_WINDOW_DAYS = 90; -const MAX_WINDOW_DAYS = 365; + * window is directly comparable to the fleet's. Exported (#9148) so the import side can range-check a peer's + * `windowDays` and the benchmark side can compare a peer's window against this instance's own, without + * duplicating the clamp. */ +export const DEFAULT_WINDOW_DAYS = 90; +export const MAX_WINDOW_DAYS = 365; + +/** Resolve the effective window (days) from a caller-supplied override, exactly as {@link buildFederatedBundle} + * does internally — extracted so {@link buildFederatedBenchmark} (federated-benchmark.ts) can compute the + * SAME value to pass to the import side's window-match check (#9148) without forking the clamp logic. */ +export function resolveFederatedWindowDays(windowDaysOverride: number | undefined): number { + return Number.isFinite(windowDaysOverride) && (windowDaysOverride as number) > 0 + ? Math.min(windowDaysOverride as number, MAX_WINDOW_DAYS) + : DEFAULT_WINDOW_DAYS; +} /** * The signed payload of a federated calibration bundle: every field except the signature itself. @@ -163,16 +175,46 @@ export function canonicalizeFederatedBundleBody(body: FederatedSignalBundleBody) * HMAC-sign a bundle body so a receiving instance can verify it was not tampered with in transit. * * KEY-TRUST (#6477, ratified as an allowlist design — NOT a PKI/reputation system): the signing key is this - * instance's dedicated anonymization secret (getOrCreateAnonSecret), which makes the bundle tamper-evident to - * anyone holding that key. Peer trust itself is established on the RECEIVING side, where #6480 has SHIPPED - * (src/orb/federated-import.ts): an importing operator explicitly adds a peer's verification key to - * `federatedIntelligence.peerKeys` (fail-closed when unset), and only an allowlisted key can verify a bundle. - * So this signature is the tamper-evidence layer; the receiver's allowlist is what turns it into peer trust. + * instance's DEDICATED federated-signing secret (getOrCreateFederatedSigningSecret) — a symmetric HMAC key a + * peer must hold verbatim to verify a bundle. #9147: this is deliberately a SEPARATE secret from the always-on + * #1255 orb export's anonymization secret (getOrCreateAnonSecret in ../selfhost/orb-collector.ts). Handing a + * peer the federated signing key must never also hand them the key that de-anonymizes this instance's + * unrelated, always-on telemetry — the two secrets share no value and are rotatable independently. Peer trust + * itself is established on the RECEIVING side, where #6480 has SHIPPED (src/orb/federated-import.ts): an + * importing operator explicitly adds a peer's verification key to `federatedIntelligence.peerKeys` + * (fail-closed when unset), and only an allowlisted key can verify a bundle. So this signature is the + * tamper-evidence layer; the receiver's allowlist is what turns it into peer trust. */ export function signFederatedBundle(body: FederatedSignalBundleBody, key: string): string { return createHmac("sha256", key).update(canonicalizeFederatedBundleBody(body)).digest("hex"); } +/** system_flags key for the federated bundle's OWN signing secret (#9147) — never `orb:anon_secret`. */ +const FEDERATED_SIGNING_SECRET_FLAG = "orb:federated_signing_secret"; + +/** + * The instance's DEDICATED federated-signing secret (#9147): a 256-bit random key generated once and + * persisted in system_flags, distinct from every other secret this codebase mints (the App private key, the + * webhook-verification secret, and — the specific bug this closes — the always-on orb export's anonymization + * secret). An operator enrolling a peer only ever hands out THIS value, so a peer can never use it to + * de-anonymize the unrelated #1255 telemetry export, and rotating either secret never affects the other. + */ +export async function getOrCreateFederatedSigningSecret(db: D1Database): Promise { + const read = async (): Promise => { + const row = await db.prepare(`SELECT value FROM system_flags WHERE key = ?`).bind(FEDERATED_SIGNING_SECRET_FLAG).first<{ value: string }>(); + return row?.value; + }; + const existing = await read(); + if (existing) return existing; + const generated = randomBytes(32).toString("hex"); // 256-bit, 64 hex chars — same shape as generateAnonSecret + await db + .prepare(`INSERT OR IGNORE INTO system_flags (key, value, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP)`) + .bind(FEDERATED_SIGNING_SECRET_FLAG, generated) + .run(); + /* v8 ignore next -- a row always exists after INSERT OR IGNORE, so the ?? fallback is unreachable */ + return (await read()) ?? generated; +} + /** Is the federated export opted in for this deployment? Absent block ⇒ false ⇒ byte-identical behavior. */ export function isFederatedIntelligenceEnabled(manifest: Pick | null | undefined): boolean { return manifest?.federatedIntelligence?.enabled === true; @@ -198,17 +240,16 @@ export async function buildFederatedBundle( if (!isFederatedIntelligenceEnabled(manifest)) return null; try { - const windowDays = - Number.isFinite(opts.windowDays) && (opts.windowDays as number) > 0 - ? Math.min(opts.windowDays as number, MAX_WINDOW_DAYS) - : DEFAULT_WINDOW_DAYS; + const windowDays = resolveFederatedWindowDays(opts.windowDays); const now = Number.isFinite(opts.now) ? (opts.now as number) : Date.now(); // Date-only cutoff, like computeFleetAnalytics — compares correctly whether created_at is ISO ('…T…Z') or // SQLite's CURRENT_TIMESTAMP space format ('YYYY-MM-DD HH:MM:SS'). const cutoff = new Date(now - windowDays * 86_400_000).toISOString().slice(0, 10); - const secret = await getOrCreateAnonSecret(db); - const instance = instanceId(secret); + // #9147: instanceId is derived from the dedicated IDENTITY secret (shared with the #1255 orb export, so + // both pipelines dedup the same instance under the same handle) — never from the anon secret or the + // federated signing secret below, so rotating either never changes this instance's identity. + const instance = instanceId(await getOrCreateInstanceIdentitySecret(db)); const { results } = await db.prepare(LOCAL_CALIBRATION_QUERY).bind(cutoff).all(); const rows = results ?? []; @@ -255,7 +296,11 @@ export async function buildFederatedBundle( copycatRate: bucketShare("duplicate_risk"), }; - return { ...body, signature: signFederatedBundle(body, secret) }; + // #9147: signed with the DEDICATED federated signing secret — never the anon secret above, and never + // shared with any other credential — so handing this key to a peer can only ever verify federated + // bundles, not de-anonymize the unrelated always-on orb export. + const signingSecret = await getOrCreateFederatedSigningSecret(db); + return { ...body, signature: signFederatedBundle(body, signingSecret) }; } catch (error) { console.error( JSON.stringify({ level: "error", event: "federated_bundle_failed", message: String(error).slice(0, 200) }), diff --git a/src/orb/federated-collector.ts b/src/orb/federated-collector.ts index 1ba1ffbe67..e3406cef05 100644 --- a/src/orb/federated-collector.ts +++ b/src/orb/federated-collector.ts @@ -28,6 +28,7 @@ import { } from "@loopover/engine"; import { isSafeHttpUrl } from "../review/content-lane/safe-url"; import { buildFederatedBundle, FEDERATED_BUNDLE_SCHEMA_VERSION, type FederatedSignalBundle } from "./federated-bundle"; +import { readOrbIngestBody } from "./ingest"; import type { FocusManifest } from "../signals/focus-manifest"; /** Matches every other outbound call in this subsystem (orb-collector.ts:215's 30s export tick). */ @@ -38,6 +39,10 @@ const DEFAULT_MAX_ATTEMPTS = 3; const RETRY_BASE_MS = 500; /** A best-effort background sync has no business hammering a peer's collector. */ const RATE_LIMIT: { limit: number; windowMs: number } = { limit: 6, windowMs: 60_000 }; +/** #9148: cap on how many pulled entries are even shape-checked, independent of the byte ceiling below — a + * hostile/misconfigured collector can't make this instance do unbounded per-element work just by staying + * under the byte cap with many tiny objects. Comfortably above any realistic peer count for this feature. */ +const MAX_PULLED_BUNDLES = 200; type ManifestSlice = Pick | null | undefined; @@ -50,11 +55,33 @@ export type CollectorOpts = { sleepFn?: (ms: number) => Promise; /** Injected random for the jitter — jitteredBackoffMs never reads Math.random itself. */ randomFn?: () => number; - /** Caller-owned rolling-window bucket. Omitted ⇒ no local rate limiting is applied. */ + /** Caller-owned rolling-window bucket. Omitted ⇒ falls back to this module's own default bucket for the + * given direction (#9166) — the limiter is never opt-in, so a caller can't accidentally forget to pass + * one and lose rate limiting entirely (which is exactly what happened before: the only production caller, + * buildFederatedBenchmark's routes.ts call site, never supplied a bucket at all). Tests may still inject + * their own bucket for isolation from the module-level default. */ bucket?: LocalRateBucket; now?: number; }; +/** #9166: module-level default buckets, one per direction, so the rate limit engages even when a caller + * supplies no `opts.bucket` of its own. `evaluateLocalRateLimit` is pure (it only reads a bucket, never + * advances it) — {@link rateLimitAllows} is what mutates these in place after an allowed attempt. */ +const defaultRateLimitBuckets: Record<"push" | "pull", LocalRateBucket> = { + push: { count: 0, windowStartMs: 0 }, + pull: { count: 0, windowStartMs: 0 }, +}; + +/** TEST-ONLY: reset the module-level default buckets between test cases. Production code never calls this — + * the whole point of the default is that it persists for the life of the isolate. Exported purely so tests in + * the same file (which otherwise share this module-level state) stay isolated from each other. */ +export function __resetFederatedCollectorRateLimitForTest(): void { + defaultRateLimitBuckets.push.count = 0; + defaultRateLimitBuckets.push.windowStartMs = 0; + defaultRateLimitBuckets.pull.count = 0; + defaultRateLimitBuckets.pull.windowStartMs = 0; +} + /** * The collector endpoint armed for `direction`, or null when this instance must not talk to anyone: not opted * in, no collector configured, the configured URL failed the SSRF guard at parse time, or the operator scoped @@ -74,10 +101,20 @@ export function resolveCollectorEndpoint(manifest: ManifestSlice, direction: "pu return url; } -/** True when the caller's bucket still permits an attempt. No bucket ⇒ unlimited. */ -function rateLimitAllows(opts: CollectorOpts, now: number): boolean { - if (!opts.bucket) return true; - return evaluateLocalRateLimit(opts.bucket, RATE_LIMIT, now).allowed; +/** True when the bucket for `direction` still permits an attempt at `now` — and, when it does, ADVANCES that + * bucket's count/window in place (#9166: evaluateLocalRateLimit itself never mutates its input, so this is + * the one place that turns an allowed decision into actual consumption). A caller-supplied `opts.bucket` + * always wins (test isolation); otherwise this module's own per-direction default bucket is used, so the + * limit is enforced even when nobody passes one in. */ +function rateLimitAllows(direction: "push" | "pull", opts: CollectorOpts, now: number): boolean { + const bucket = opts.bucket ?? defaultRateLimitBuckets[direction]; + const decision = evaluateLocalRateLimit(bucket, RATE_LIMIT, now); + if (decision.allowed) { + const windowElapsed = now - bucket.windowStartMs >= RATE_LIMIT.windowMs; + bucket.count = (windowElapsed ? 0 : bucket.count) + 1; + if (windowElapsed) bucket.windowStartMs = now; + } + return decision.allowed; } /** A 4xx is the operator's own misconfiguration and will fail identically on a retry; only 5xx/network is @@ -140,7 +177,7 @@ export async function pushFederatedBundle(manifest: ManifestSlice, db: D1Databas try { const now = Number.isFinite(opts.now) ? (opts.now as number) : Date.now(); - if (!rateLimitAllows(opts, now)) return false; + if (!rateLimitAllows("push", opts, now)) return false; const bundle = await buildFederatedBundle(manifest, db, opts.now === undefined ? {} : { now: opts.now }); // The builder already fails safe to null; nothing to send is not a failure worth retrying. @@ -168,6 +205,11 @@ export async function pushFederatedBundle(manifest: ManifestSlice, db: D1Databas * signature-verified or trust-gated — that is #6480's job, shipped in ./federated-import.ts (the peerKeys * allowlist). Returns [] rather than throwing on any failure, so an unreachable or hostile collector is * indistinguishable from "no peers yet" to every caller. + * + * #9148: the response body is read through the SAME bounded reader the /v1/orb/ingest path uses + * (readOrbIngestBody — both a Request and a Response satisfy its structural input type), so a hostile or + * misconfigured collector can't make this instance buffer an unbounded response; the parsed array is then + * capped at MAX_PULLED_BUNDLES before any per-element shape check runs. */ export async function pullPeerBundles(manifest: ManifestSlice, opts: CollectorOpts = {}): Promise { const endpoint = resolveCollectorEndpoint(manifest, "pull"); @@ -175,14 +217,21 @@ export async function pullPeerBundles(manifest: ManifestSlice, opts: CollectorOp try { const now = Number.isFinite(opts.now) ? (opts.now as number) : Date.now(); - if (!rateLimitAllows(opts, now)) return []; + if (!rateLimitAllows("pull", opts, now)) return []; const response = await fetchWithRetry(endpoint, { method: "GET", headers: { accept: "application/json" } }, opts); if (response === null) return []; - const payload: unknown = await response.json(); + const text = await readOrbIngestBody(response, response.headers.get("content-length")); + if (!text) return []; + let payload: unknown; + try { + payload = JSON.parse(text); + } catch { + return []; + } if (!Array.isArray(payload)) return []; - return payload.filter(isBundleShaped); + return payload.slice(0, MAX_PULLED_BUNDLES).filter(isBundleShaped); } catch (error) { console.error( JSON.stringify({ level: "error", event: "federated_pull_failed", message: String(error).slice(0, 200) }), diff --git a/src/orb/federated-import.ts b/src/orb/federated-import.ts index 228f76d4e9..6ef784f8a5 100644 --- a/src/orb/federated-import.ts +++ b/src/orb/federated-import.ts @@ -4,26 +4,59 @@ // (src/orb/federated-collector.ts, #6479) may be folded into local calibration or the peer-median benchmark // (#6481) at all. The export side is #6478 (src/orb/federated-bundle.ts). // -// The trust model is #6477's DESIGN DECISION, implemented here exactly as specified and deliberately NOT -// redesigned. Its two poisoning-resistance layers, and where each one actually lives: +// The trust model is #6477's DESIGN DECISION. Its poisoning-resistance layers, and where each one lives: // 1. ALLOWLIST — only a peer whose verification key the operator explicitly added to -// `federatedIntelligence.peerKeys` is ever considered. That is enforced HERE, and it is why a Sybil -// attack is self-limiting by construction: forging peers requires the RECEIVING operator to have added -// the attacker's keys themselves. Mirrors MCP_READ_REPO_ALLOWLIST's posture: explicit operator config, -// fail closed when unset, never auto-discovery and never a PKI. +// `federatedIntelligence.peerKeys` is ever considered. Mirrors MCP_READ_REPO_ALLOWLIST's posture: +// explicit operator config, fail closed when unset, never auto-discovery and never a PKI. // 2. MEDIAN, NOT MEAN — a bounded number of outliers cannot drag a median arbitrarily, unlike a mean. That // layer needs no code here: the fleet aggregation this feeds already medians (src/orb/analytics.ts:92), // so it holds by construction. Re-implementing it in this module would fork the definition #6481's // comparison depends on. +// 3. PER-KEY SYBIL CAP + PERSISTED WATERMARK (#9148) — layer 1 alone is NOT self-limiting for peers, only +// for KEYS: nothing previously bound an `instanceId` to a key, so one allowlisted key could sign an +// unbounded number of distinct fabricated `instanceId`s and, since `accepted` had no per-instance dedup +// either, own the entire median outright. {@link applyFederatedPeerWatermarks} closes both gaps: it +// keeps only the LAST bundle per `instanceId` in a batch, persists a per-instance high-water mark +// (`generatedAt`) so an old-but-still-fresh bundle can never be replayed forever, and caps how many +// distinct instanceIds a single verifying key may ever contribute. // // #6477 explicitly rejected building a reputation/decay/scoring system for trust, so there is deliberately no // per-peer score, no anomaly heuristic, and no retroactive poisoned-bundle detection here: an operator who -// discovers a bad peer removes its key from the allowlist. Adding any of those would be inventing a mechanism -// that design pass considered and turned down. -import { canonicalizeFederatedBundleBody, FEDERATED_BUNDLE_SCHEMA_VERSION, type FederatedSignalBundle, type FederatedSignalBundleBody } from "./federated-bundle"; +// discovers a bad peer removes its key from the allowlist. The Sybil cap above is a bound on how much damage +// one STILL-TRUSTED key can do, not a reputation system for individual peers. +import { createHash, createHmac } from "node:crypto"; +import { MIN_DECIDED } from "./analytics"; +import { + canonicalizeFederatedBundleBody, + FEDERATED_BUNDLE_SCHEMA_VERSION, + resolveFederatedWindowDays, + type FederatedSignalBundle, + type FederatedSignalBundleBody, +} from "./federated-bundle"; import { timingSafeEqualHex } from "../utils/crypto"; import type { FocusManifest } from "../signals/focus-manifest"; -import { createHmac } from "node:crypto"; + +/** A pulled bundle older than this (relative to `now`) is rejected rather than counted forever — closes the + * "re-serve one favorable year-old bundle" hole (#9148). Generous relative to DEFAULT_WINDOW_DAYS so a + * best-effort background sync that occasionally misses a tick for a day or two never starts rejecting a + * peer that is still perfectly healthy. */ +const MAX_BUNDLE_AGE_MS = 7 * 86_400_000; + +/** How far into the future a `generatedAt` may claim to be before it's rejected outright, rather than merely + * accepted with a skewed clock. Generous enough to absorb real clock drift between two independent + * self-hosted instances without opening a meaningful backdating/replay window. */ +const MAX_CLOCK_SKEW_MS = 5 * 60_000; + +/** How many DISTINCT `instanceId`s a single verifying key may ever contribute to the accepted set (#9148). + * Without this, one allowlisted key can mint an unbounded number of fabricated instanceIds and dominate the + * peer median outright — the allowlist bounds which KEYS are trusted, not how many PEERS a key may claim to + * speak for. Deliberately generous: this is a ceiling on abuse, not a realistic peer-count expectation for + * a self-hosted federation of this scale. */ +const MAX_INSTANCES_PER_KEY = 10; + +/** Peer-state entries not refreshed in this long are pruned on the next tick, so a key that stops presenting + * a stale instanceId eventually frees its slot under the cap above, rather than permanently consuming it. */ +const PEER_STATE_PRUNE_AFTER_MS = 400 * 86_400_000; /** Why a bundle was not folded in. Every rejection carries one of these, so a rejection is always traceable to * a specific rule rather than vanishing silently (#6480 requires rejections be operator-visible). */ @@ -39,7 +72,27 @@ export type FederatedRejectionReason = /** No allowlisted key reproduces the signature: either an untrusted peer or a tampered body. These are * deliberately ONE reason — with a detached HMAC the receiver cannot distinguish them, and pretending * otherwise would report a distinction this scheme cannot actually make. */ - | "untrusted_or_tampered"; + | "untrusted_or_tampered" + /** A signature-covered numeric field is outside its valid range (e.g. a rate outside [0,1], a negative + * count) (#9148). Distinct from "malformed": the field has the RIGHT TYPE, just an impossible value. */ + | "out_of_range" + /** `windowDays` does not match the LOCAL instance's own resolved window, so the values are not comparable — + * #9148, closing "mixing windowDays: 7 with windowDays: 365 bundles into one median is uncaught". */ + | "window_mismatch" + /** `generatedAt` is unparseable, too far in the future for plausible clock skew, or older than + * MAX_BUNDLE_AGE_MS — #9148, closing "re-serve one favorable year-old bundle forever". */ + | "stale_or_future" + /** `decided` is below MIN_DECIDED — the sender's own eligibility bar, enforced receiver-side rather than + * trusted from the sender (#9148: nothing previously stopped a peer self-reporting `decided: 3, + * mergePrecision: 1.0`). */ + | "below_min_decided" + /** This exact `instanceId` already has a NEWER `generatedAt` on record — a replay or clock-rollback of an + * otherwise-valid bundle (#9148, the persisted high-water mark). Only ever produced by + * {@link applyFederatedPeerWatermarks}, which is the one place this pipeline touches the DB. */ + | "replayed_or_rollback" + /** This `instanceId` is new, and admitting it would push its verifying key over MAX_INSTANCES_PER_KEY — + * the per-key Sybil cap (#9148). Only ever produced by {@link applyFederatedPeerWatermarks}. */ + | "sybil_cap_exceeded"; /** One rejected bundle, reduced to what an operator can act on without leaking bundle contents. */ export interface FederatedRejection { @@ -71,7 +124,13 @@ export function isFederatedImportEnabled(manifest: ManifestSlice | null | undefi } /** Does `bundle` carry every signature-covered field, with the right type? Guards the canonicalization below: - * an absent field would otherwise serialize as `undefined` and silently change the signed bytes. */ + * an absent field would otherwise serialize as `undefined` and silently change the signed bytes. + * + * TYPE-SHAPE ONLY (#9148: split from the RANGE check below on purpose) — a field can be a finite number of + * the wrong magnitude (a rate of -3, a decided count of -1) and still pass this function; that is + * {@link isBundleBodyInRange}'s job. Keeping the two separate lets a caller distinguish "malformed" (wrong + * type — likely a schema mismatch) from "out_of_range" (right type, impossible value — likely a hostile or + * buggy peer) in the rejection reason it reports. */ function isBundleBodyShaped(bundle: FederatedSignalBundle): boolean { const numeric = (value: unknown): boolean => typeof value === "number" && Number.isFinite(value); const nullableNumeric = (value: unknown): boolean => value === null || numeric(value); @@ -93,6 +152,24 @@ function isBundleBodyShaped(bundle: FederatedSignalBundle): boolean { ); } +/** Is every numeric field within the range its own semantics allow? Only called once {@link isBundleBodyShaped} + * has already confirmed every field is a finite number (or, for the nullable ones, null) — this function + * never needs to re-guard against `NaN`/`Infinity`/wrong-typeof itself. A rate field outside [0, 1], a + * negative count, or `cycleP50Ms > cycleP95Ms` (the median can never exceed the p95 of the same sample) all + * fail here (#9148). */ +function isBundleBodyInRange(bundle: FederatedSignalBundle): boolean { + const unitRate = (value: number | null): boolean => value === null || (value >= 0 && value <= 1); + const nonNegative = (value: number | null): boolean => value === null || value >= 0; + if (bundle.windowDays <= 0) return false; + if (bundle.decided < 0) return false; + if (!unitRate(bundle.reversalRate) || !unitRate(bundle.slopRate) || !unitRate(bundle.copycatRate)) return false; + if (!unitRate(bundle.mergePrecision) || !unitRate(bundle.closePrecision)) return false; + if (!unitRate(bundle.fpRate) || !unitRate(bundle.fnRate)) return false; + if (!nonNegative(bundle.cycleP50Ms) || !nonNegative(bundle.cycleP95Ms)) return false; + if (bundle.cycleP50Ms !== null && bundle.cycleP95Ms !== null && bundle.cycleP50Ms > bundle.cycleP95Ms) return false; + return true; +} + /** Strip the detached signature back off, so the body is canonicalized over exactly the fields the sender * signed. Rebuilt field-by-field rather than by deleting `signature` from a copy: the canonical form is a * fixed key list, so an extra property a peer appended can never reach the signed bytes. */ @@ -116,45 +193,67 @@ function toBody(bundle: FederatedSignalBundle): FederatedSignalBundleBody { } /** - * Does `bundle`'s signature verify against ANY key the operator allowlisted? + * Which allowlisted key (if any) verifies `bundle`'s signature — the value itself, not an index, since + * `peerKeys` order is operator config and not a stable identity (#9148's per-key Sybil-cap fingerprinting + * needs a value stable across config edits that don't touch that specific key). * * Every candidate key is tried because the HMAC is detached and carries no key hint — the bundle says which * INSTANCE it claims to be from, but `instanceId` is unauthenticated until a key verifies, so selecting a key * by it would trust the attacker-controlled field to pick its own verifier. * * The comparison is timing-safe (timingSafeEqualHex), and the loop deliberately does NOT early-exit on a match: - * it verifies against all keys and ORs the results, so total work does not depend on WHICH key matched. + * it verifies against all keys, so total work does not depend on WHICH key matched (or whether one did). */ -export function verifyFederatedBundle(bundle: FederatedSignalBundle, peerKeys: readonly string[]): boolean { +export function matchingFederatedKey(bundle: FederatedSignalBundle, peerKeys: readonly string[]): string | null { const canonical = canonicalizeFederatedBundleBody(toBody(bundle)); - let verified = false; + let matched: string | null = null; for (const key of peerKeys) { const expected = createHmac("sha256", key).update(canonical).digest("hex"); - if (timingSafeEqualHex(bundle.signature, expected)) verified = true; + if (timingSafeEqualHex(bundle.signature, expected)) matched = key; } - return verified; + return matched; +} + +/** Does `bundle`'s signature verify against ANY key the operator allowlisted? Thin boolean wrapper over + * {@link matchingFederatedKey} — kept as its own export because most callers (and every existing test) only + * need the yes/no answer, not which key matched. */ +export function verifyFederatedBundle(bundle: FederatedSignalBundle, peerKeys: readonly string[]): boolean { + return matchingFederatedKey(bundle, peerKeys) !== null; } -/** Apply every gate to a single bundle. Returns null when it may be folded in, or the reason it may not. */ -function rejectionFor(bundle: FederatedSignalBundle, peerKeys: readonly string[]): FederatedRejectionReason | null { +/** Apply every STATELESS gate to a single bundle (no DB, no network) — schema, shape, range, signature, + * freshness, and window-match. Returns null when it may proceed to the persisted per-instance gates in + * {@link applyFederatedPeerWatermarks}, or the reason it may not. */ +function rejectionFor(bundle: FederatedSignalBundle, peerKeys: readonly string[], now: number, localWindowDays: number): FederatedRejectionReason | null { if (bundle?.schemaVersion !== FEDERATED_BUNDLE_SCHEMA_VERSION) return "unsupported_schema_version"; if (!isBundleBodyShaped(bundle)) return "malformed"; + if (!isBundleBodyInRange(bundle)) return "out_of_range"; if (!verifyFederatedBundle(bundle, peerKeys)) return "untrusted_or_tampered"; + if (bundle.windowDays !== localWindowDays) return "window_mismatch"; + const generatedAtMs = Date.parse(bundle.generatedAt); + if (!Number.isFinite(generatedAtMs)) return "stale_or_future"; + if (now - generatedAtMs > MAX_BUNDLE_AGE_MS || generatedAtMs - now > MAX_CLOCK_SKEW_MS) return "stale_or_future"; + if (bundle.decided < MIN_DECIDED) return "below_min_decided"; return null; } /** * Trust-gate a batch of pulled peer bundles, returning only those an operator's own config says to trust. * - * FAIL-SAFE: this is a pure function the gate never consults — it reads no DB, makes no network call, and - * returns a value rather than mutating anything, so neither a rejected nor a malformed bundle can reach this - * instance's own review/merge behavior. That is the structural version of #6480's fail-safe requirement: there - * is no path from here to a gate decision, rather than a guard that could be forgotten. + * FAIL-SAFE, MOSTLY PURE: this function reads no DB and makes no network call — the gate never consults it, + * so neither a rejected nor a malformed bundle can reach this instance's own review/merge behavior. (The one + * exception in this module is {@link applyFederatedPeerWatermarks}, a separate function that DOES touch the + * DB, called only from the federated background sync job — never from any gate-facing path either.) + * + * Applies every stateless gate (#9148: schema, shape, range, signature, freshness, and window-match) and + * dedups within the batch by `instanceId`, keeping only the LAST bundle for each — closing the other half of + * the Sybil gap: a single verifying key signing many bundles for the SAME fabricated instanceId in one pull + * no longer counts multiple times toward the median. */ export function importPeerBundles( manifest: ManifestSlice | null | undefined, bundles: readonly FederatedSignalBundle[], - opts: { log?: FederatedImportLogger } = {}, + opts: { log?: FederatedImportLogger; now?: number; localWindowDays?: number } = {}, ): FederatedImportResult { const log = opts.log ?? defaultRejectionLogger; const reject = (instanceId: string | null, reason: FederatedRejectionReason): FederatedRejection => { @@ -173,14 +272,23 @@ export function importPeerBundles( return { accepted: [], rejected: bundles.map((bundle) => reject(instanceIdOf(bundle), "no_trusted_peers")) }; } - const accepted: FederatedSignalBundle[] = []; + const now = Number.isFinite(opts.now) ? (opts.now as number) : Date.now(); + const localWindowDays = Number.isFinite(opts.localWindowDays) ? (opts.localWindowDays as number) : resolveFederatedWindowDays(undefined); + const rejected: FederatedRejection[] = []; + // Keep only the LAST bundle per instanceId (a Map preserves insertion order but re-set moves nothing, so we + // delete-then-set to push a re-seen id to the end — irrelevant to correctness here since only the VALUE at + // each key is read back below, never iteration order, but matches the "last wins" contract literally). + const acceptedByInstance = new Map(); for (const bundle of bundles) { - const reason = rejectionFor(bundle, config.peerKeys); - if (reason === null) accepted.push(bundle); - else rejected.push(reject(instanceIdOf(bundle), reason)); + const reason = rejectionFor(bundle, config.peerKeys, now, localWindowDays); + if (reason !== null) { + rejected.push(reject(instanceIdOf(bundle), reason)); + continue; + } + acceptedByInstance.set(bundle.instanceId, bundle); } - return { accepted, rejected }; + return { accepted: [...acceptedByInstance.values()], rejected }; } /** The claimed handle, or null when the bundle is too malformed to carry one. Unauthenticated until a @@ -189,6 +297,124 @@ function instanceIdOf(bundle: FederatedSignalBundle): string | null { return typeof bundle?.instanceId === "string" ? bundle.instanceId : null; } +/** system_flags key for the persisted per-instance peer state (#9148): a single JSON blob keyed by + * instanceId, mirroring the get-or-create secret pattern this subsystem already uses elsewhere + * (federated-bundle.ts's FEDERATED_SIGNING_SECRET_FLAG). A dedicated table was deliberately NOT added: a + * self-hosted federation's peer count is expected to stay in the tens at most, so scanning/filtering the + * whole blob in JS on each tick is trivially cheap and avoids a migration for what is bookkeeping state, not + * a queryable relation. Revisit with a real table + index if that scale assumption ever stops holding. */ +const FEDERATED_PEER_STATE_FLAG_KEY = "orb:federated_peer_state"; + +interface FederatedPeerStateEntry { + /** SHA-256 hex of the verifying key, so the Sybil cap can be enforced without persisting the key itself. */ + keyFingerprint: string; + /** Epoch ms of the newest `generatedAt` accepted for this instanceId — the replay/rollback watermark. */ + lastGeneratedAtMs: number; + /** Epoch ms this entry was last refreshed — drives PEER_STATE_PRUNE_AFTER_MS. */ + lastSeenAtMs: number; + /** Epoch ms this instanceId was first admitted — informational only, never read for a gating decision. */ + firstSeenAtMs: number; +} + +type FederatedPeerState = Record; + +/** SHA-256 hex of a peer verification key — a stable handle for the Sybil cap that never persists the key + * value itself (mirrors this codebase's convention of never logging/storing a raw credential). */ +function federatedKeyFingerprint(key: string): string { + return createHash("sha256").update(key).digest("hex"); +} + +/** Reads the persisted peer-state blob. Fails safe to `{}` on any error (missing row, corrupt JSON, a D1 + * outage) — an unreadable cache degrades to "every instanceId looks new", never to a thrown error, matching + * every other read helper in this subsystem. */ +async function readFederatedPeerState(db: D1Database): Promise { + try { + const row = await db.prepare("SELECT value FROM system_flags WHERE key = ?").bind(FEDERATED_PEER_STATE_FLAG_KEY).first<{ value: string }>(); + if (!row?.value) return {}; + const parsed: unknown = JSON.parse(row.value); + return parsed !== null && typeof parsed === "object" && !Array.isArray(parsed) ? (parsed as FederatedPeerState) : {}; + } catch { + return {}; + } +} + +/** Best-effort write of the peer-state blob. A write failure must never fail the sync tick — the next tick + * simply recomputes from a stale-but-still-usable snapshot. */ +async function writeFederatedPeerState(db: D1Database, state: FederatedPeerState): Promise { + await db + .prepare("INSERT OR REPLACE INTO system_flags (key, value, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP)") + .bind(FEDERATED_PEER_STATE_FLAG_KEY, JSON.stringify(state)) + .run() + .catch(() => undefined); +} + +/** + * Applies the two PERSISTED gates #9148 requires on top of {@link importPeerBundles}'s stateless result: a + * per-instance replay/rollback watermark, and a per-key Sybil cap for brand-new instanceIds. This is the ONE + * function in this module that touches the DB — deliberately kept separate from `importPeerBundles` (which + * stays pure) and called ONLY from the federated background sync job (federated-sync.ts), never from any + * gate-facing path, so the "no path from here to a gate decision" property importPeerBundles documents still + * holds structurally for the pure core. + * + * `peerKeys` must be the SAME allowlist `result.accepted` was already verified against — every bundle in + * `result.accepted` is therefore guaranteed to match one of them (the `null` arm below is defensive only). + */ +export async function applyFederatedPeerWatermarks( + db: D1Database, + result: FederatedImportResult, + peerKeys: readonly string[], + opts: { now?: number; log?: FederatedImportLogger } = {}, +): Promise { + const log = opts.log ?? defaultRejectionLogger; + const now = Number.isFinite(opts.now) ? (opts.now as number) : Date.now(); + const reject = (instanceId: string | null, reason: FederatedRejectionReason): FederatedRejection => { + const rejection: FederatedRejection = { instanceId, reason }; + log(rejection); + return rejection; + }; + + const state = await readFederatedPeerState(db); + for (const [id, entry] of Object.entries(state)) { + if (now - entry.lastSeenAtMs > PEER_STATE_PRUNE_AFTER_MS) delete state[id]; + } + + const accepted: FederatedSignalBundle[] = []; + const rejected: FederatedRejection[] = [...result.rejected]; + for (const bundle of result.accepted) { + const key = matchingFederatedKey(bundle, peerKeys); + /* v8 ignore next 4 -- defensive: every bundle in result.accepted already verified against this exact + peerKeys allowlist inside importPeerBundles, so a re-check here can only fail if the caller passed a + DIFFERENT allowlist than it used to produce `result` — a caller contract violation, not a reachable + runtime state. */ + if (key === null) { + rejected.push(reject(bundle.instanceId, "untrusted_or_tampered")); + continue; + } + const fingerprint = federatedKeyFingerprint(key); + const generatedAtMs = Date.parse(bundle.generatedAt); // already validated finite by importPeerBundles' rejectionFor + const existing = state[bundle.instanceId]; + if (existing) { + if (generatedAtMs <= existing.lastGeneratedAtMs) { + rejected.push(reject(bundle.instanceId, "replayed_or_rollback")); + continue; + } + state[bundle.instanceId] = { keyFingerprint: fingerprint, lastGeneratedAtMs: generatedAtMs, lastSeenAtMs: now, firstSeenAtMs: existing.firstSeenAtMs }; + accepted.push(bundle); + continue; + } + const liveInstancesForKey = Object.values(state).filter((entry) => entry.keyFingerprint === fingerprint).length; + if (liveInstancesForKey >= MAX_INSTANCES_PER_KEY) { + rejected.push(reject(bundle.instanceId, "sybil_cap_exceeded")); + continue; + } + state[bundle.instanceId] = { keyFingerprint: fingerprint, lastGeneratedAtMs: generatedAtMs, lastSeenAtMs: now, firstSeenAtMs: now }; + accepted.push(bundle); + } + + await writeFederatedPeerState(db, state); + return { accepted, rejected }; +} + /** Operator-visible by default. Logs the reason and the opaque instance handle only — never bundle contents, * never a peer key, so a rejection is diagnosable without the log becoming a place secrets leak. */ function defaultRejectionLogger(rejection: FederatedRejection): void { diff --git a/src/orb/ingest.ts b/src/orb/ingest.ts index 1764970394..266b4a5bf1 100644 --- a/src/orb/ingest.ts +++ b/src/orb/ingest.ts @@ -26,17 +26,23 @@ function parseContentLength(header: string | null | undefined): number | null { return Number.isInteger(n) && n >= 0 ? n : null; } -/** Read the request body with a hard byte ceiling so a hostile sender can't make us buffer unbounded +/** The minimal shape {@link readOrbIngestBody} needs from its input: a byte stream. Both `Request` and + * `Response` satisfy this structurally (the Fetch API gives both a `.body: ReadableStream | + * null`), so this reader works unmodified over either — #9148 reuses it for a collector `Response` body + * (src/orb/federated-collector.ts's pullPeerBundles) instead of writing a second bounded reader. */ +export type BoundedBodySource = { body: ReadableStream | null }; + +/** Read a request/response body with a hard byte ceiling so a hostile sender can't make us buffer unbounded * input. Returns null when the body exceeds MAX_ORB_INGEST_BODY_BYTES OR when the underlying stream * itself errors (a dropped connection / network reset mid-read, mirrors readOrbRelayRegisterBody in - * ../orb/relay.ts) — both callers (/v1/orb/ingest, /v1/ams/ingest) already treat null identically to - * "reject this request", so a transient read failure degrades the same way an oversized payload does, - * instead of throwing UNCAUGHT out of this function as a bare framework 500. */ -export async function readOrbIngestBody(request: Request, contentLengthHeader: string | null | undefined): Promise { + * ../orb/relay.ts) — every caller (/v1/orb/ingest, /v1/ams/ingest, and #9148's federated-collector pull) + * already treats null identically to "reject this", so a transient read failure degrades the same way an + * oversized payload does, instead of throwing UNCAUGHT out of this function as a bare framework 500. */ +export async function readOrbIngestBody(source: BoundedBodySource, contentLengthHeader: string | null | undefined): Promise { const declared = parseContentLength(contentLengthHeader); if (declared !== null && declared > MAX_ORB_INGEST_BODY_BYTES) return null; - const stream = request.body; + const stream = source.body; if (!stream) return ""; const reader = stream.getReader(); const decoder = new TextDecoder(); diff --git a/src/orb/oauth.ts b/src/orb/oauth.ts index 88d23bdfae..4fe9a069f1 100644 --- a/src/orb/oauth.ts +++ b/src/orb/oauth.ts @@ -15,7 +15,7 @@ import type { Context } from "hono"; import { PRODUCT_USER_AGENT, timeoutFetch } from "../github/client"; import { LOOPOVER_SITE_URL } from "../github/footer"; -import { isOrbBrokerEnabled, issueOrbEnrollment } from "./broker"; +import { countLiveEnrollmentsForInstallation, isOrbBrokerEnabled, issueOrbEnrollment, ORB_SECRET_TYPE_GITHUB_TOKEN, revokeAllLiveEnrollmentsForInstallation } from "./broker"; type GitHubUser = { login: string; id?: number }; type GitHubOrgMembership = { role?: string; state?: string; organization?: { id?: number } }; @@ -67,7 +67,39 @@ export async function verifyInstallationAdmin( return body.state === "active" && body.role === "admin" && body.organization?.id === accountId; } -async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number): Promise { +/** What a maintainer landing on the OAuth callback is asking to do, and which installation it's bound to. + * `installation_id` normally comes from the GitHub-controlled query string (the install/update Setup-URL + * redirect); `rotate`/`revoke` have no such channel (GitHub only ever echoes back `code` + `state` on a bare + * `login/oauth/authorize` bounce, not arbitrary query params we didn't put there ourselves), so those two + * actions are carried in `state` instead, as `":rotate"` / `":revoke"` (#9149) + * — the value {@link orbOAuthAuthorizeUrl} puts in the "Rotate" / "Revoke all" links on the secret page. */ +type OrbOAuthIntent = { installationId: number | null; action: "enroll" | "rotate" | "revoke" }; + +function parseOAuthState(raw: string | undefined): OrbOAuthIntent { + if (!raw) return { installationId: null, action: "enroll" }; + const [idPart, actionPart] = raw.split(":"); + const parsedId = Number(idPart); + const installationId = Number.isInteger(parsedId) && parsedId > 0 ? parsedId : null; + const action = actionPart === "rotate" ? "rotate" : actionPart === "revoke" ? "revoke" : "enroll"; + return { installationId, action }; +} + +/** A `login/oauth/authorize` link that lands the maintainer back on THIS SAME callback with a fresh, single- + * use `code` and `state=:` (#9149) — the only way to re-prove admin-of-installation + * for a rotate/revoke action without our own persisted maintainer session. Null when the client id isn't + * configured (broker effectively unusable anyway), so the caller can omit the link entirely rather than + * point at a URL that can never work. */ +function orbOAuthAuthorizeUrl(env: Env, installationId: number, action: "rotate" | "revoke"): string | null { + /* v8 ignore next 2 -- defensive: every caller of this function is already past exchangeOrbOAuthCode, which + itself returns null (→ the identity-error page, never reaching secretPage) without ORB_GITHUB_CLIENT_ID + set — so this branch can't be live-reached through the real callback flow. Kept so a future change that + loosens that upstream guard degrades to an omitted link rather than a broken one. */ + if (!env.ORB_GITHUB_CLIENT_ID) return null; + const params = new URLSearchParams({ client_id: env.ORB_GITHUB_CLIENT_ID, state: `${installationId}:${action}` }); + return `https://github.com/login/oauth/authorize?${params.toString()}`; +} + +async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number, action: "enroll" | "rotate" | "revoke" = "enroll"): Promise { // A thrown network error (DNS failure, or a timeout past timeoutFetch's own retry budget) from any of the three // GitHub calls below degrades to the same clean landing page a bad HTTP *response* already produces, instead of // escaping handleOrbOAuthCallback as an uncaught framework 500. Mirrors the failure-doesn't-escape convention in @@ -102,6 +134,14 @@ async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, return identityError(); } if (!isAdmin) return c.html(landingPage(c.env, "Admin access required", "You must be an admin of this installation's account to enroll it for self-host."), 403); + // #9149: a REVOKE request skips the active/disabled checks below on purpose — a maintainer revoking a + // leaked secret must be able to do so EVEN ON an installation the operator has since suspended or disabled; + // there is no reason unrelated administrative state should block someone locking down their own credential. + // It also never touches `registered` (no auto-registration side effect for a request that isn't enrolling). + if (action === "revoke") { + const revokedCount = await revokeAllLiveEnrollmentsForInstallation(c.env, installationId); + return c.html(revokedPage(revokedCount)); + } if (install.removed_at !== null || install.suspended_at !== null) return c.html(landingPage(c.env, "Installation not active", "This installation is suspended or uninstalled — re-install the Orb App, then retry."), 403); if (install.self_enrollment_disabled === 1) return c.html(landingPage(c.env, "Installation disabled", "This installation was disabled by the operator — contact the operator to re-enable self-host enrollment."), 403); // Zero-touch self-service: a verified admin of an ACTIVE, non-disabled install self-registers it (registered=1). @@ -109,19 +149,29 @@ async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, if (install.registered !== 1) { await c.env.DB.prepare("UPDATE orb_github_installations SET registered = 1, last_event_at = CURRENT_TIMESTAMP WHERE installation_id = ?").bind(installationId).run(); } - const result = await issueOrbEnrollment(c.env, installationId, { login: user.login, githubId: user.id ?? null }); + const result = await issueOrbEnrollment(c.env, installationId, { login: user.login, githubId: user.id ?? null }, ORB_SECRET_TYPE_GITHUB_TOKEN, { rotate: action === "rotate" }); /* v8 ignore next -- defensive: the existence + admin + active checks above passed and we just set registered=1, so issueOrbEnrollment (which re-checks existence + registered) cannot return an error here; kept to degrade safely. */ if ("error" in result) return c.html(landingPage(c.env, "Couldn't issue an enrollment", "Please retry, or contact the operator."), 409); - return c.html(secretPage(result.secret)); + const liveCount = await countLiveEnrollmentsForInstallation(c.env, installationId); + c.header("Cache-Control", "no-store"); // #9149: this response body is a plaintext secret shown exactly once — never cacheable + return c.html(secretPage(c.env, result.secret, installationId, liveCount)); } export async function handleOrbOAuthCallback(c: Context<{ Bindings: Env }>): Promise { const code = c.req.query("code"); - const installationId = Number(c.req.query("installation_id")); - // Self-enrollment: a maintainer authorized with an OAuth code + an installation_id, and the broker is enabled. - if (code && Number.isInteger(installationId) && installationId > 0 && isOrbBrokerEnabled(c.env)) { - return handleOrbEnrollment(c, code, installationId); + // #9149: installation_id normally arrives in the query string (GitHub's own install/update Setup-URL + // redirect); a rotate/revoke request instead carries it inside `state` (see parseOAuthState's doc comment + // for why) — the query value, when itself valid, still wins, so a genuine install/update redirect is never + // second-guessed by a stray/malformed `state`. + const state = parseOAuthState(c.req.query("state")); + const rawInstallationId = Number(c.req.query("installation_id")); + const queryInstallationId = Number.isInteger(rawInstallationId) && rawInstallationId > 0 ? rawInstallationId : null; + const installationId = queryInstallationId ?? state.installationId; + // Self-enrollment (or a rotate/revoke of one): a maintainer authorized with an OAuth code + a resolved + // installation_id, and the broker is enabled. + if (code && installationId !== null && isOrbBrokerEnabled(c.env)) { + return handleOrbEnrollment(c, code, installationId, state.action); } const updated = c.req.query("setup_action") === "update"; return c.html( @@ -146,10 +196,30 @@ function landingPage(env: Env, heading: string, message: string): string { } /** Show the freshly-issued enrollment secret ONCE. The secret is a generated opaque token (no user input), safe - * to embed; it is never logged. */ -function secretPage(secret: string): string { + * to embed; it is never logged. #9149: also surfaces how many enrollments are now live for this installation + * (an operator/maintainer previously had no way to see this had accumulated) and, when the client id is + * configured, links to re-authorize with a `state`-carried rotate/revoke intent (see orbOAuthAuthorizeUrl) — + * a self-hoster otherwise had no reachable path to either action at all. `installationId` is never + * user-supplied at render time (bound server-side, same as the enrollment itself), so it is safe to embed. */ +function secretPage(env: Env, secret: string, installationId: number, liveCount: number): string { + const rotateUrl = orbOAuthAuthorizeUrl(env, installationId, "rotate"); + const revokeUrl = orbOAuthAuthorizeUrl(env, installationId, "revoke"); + const manageSection = + rotateUrl && revokeUrl + ? `

This installation now has ${liveCount} live enrollment secret${liveCount === 1 ? "" : "s"}, including this one. Rotate (revokes every other one and issues this as the only valid secret) or revoke all if you no longer need self-host access for this installation.

` + : ""; return shell( "Your enrollment secret", - `

Set this as ORB_ENROLLMENT_SECRET in your self-host .env, then restart the container. It is shown once — store it now.

${secret}
`, + `

Set this as ORB_ENROLLMENT_SECRET in your self-host .env, then restart the container. It is shown once — store it now.

${secret}
${manageSection}`, + ); +} + +/** Confirms a revoke-all action (#9149) — the self-hoster-reachable counterpart to secretPage. `count` is + * never user-supplied (it's the return of revokeAllLiveEnrollmentsForInstallation), so it's safe to embed. */ +function revokedPage(count: number): string { + const plural = count === 1 ? "secret has" : "secrets have"; + return shell( + "Enrollment secrets revoked", + `

${count} live enrollment ${plural} been revoked for this installation. Any self-hosted container still using ${count === 1 ? "it" : "one of them"} loses broker access immediately. Re-run the install flow to issue a new one.

`, ); } diff --git a/src/orb/relay.ts b/src/orb/relay.ts index 924b63e7f0..8c48d9c2ff 100644 --- a/src/orb/relay.ts +++ b/src/orb/relay.ts @@ -9,6 +9,7 @@ import { githubWebhookCoalesceKey } from "../github/webhook-coalesce"; import { isSafeHttpUrl } from "../review/content-lane/safe-url"; import type { GitHubWebhookPayload } from "../types"; import { decryptSecret, encryptSecret } from "../utils/crypto"; +import { incr } from "../selfhost/metrics"; // The events a brokered container needs to review/act on. Installation-lifecycle + other Orb-internal events are // deliberately NOT forwarded (the container runs under the CENTRAL Orb App, not its own, so it must not treat @@ -322,18 +323,24 @@ export async function pruneRelayPending(env: Env): Promise { /** Enqueue a pull-mode event for an installation. Idempotent on delivery_id (mirrors storeRelayFailure) — a GitHub * redelivery reaching the Orb twice never double-queues. Prunes expired rows and caps each install's backlog first so - * an offline/malicious pull enrollment cannot retain raw webhook bodies indefinitely. */ + * an offline/malicious pull enrollment cannot retain raw webhook bodies indefinitely. + * + * `enrollId` (#9150) tags the row with the specific enrollment forwardOrbEvent picked as the current winner + * for this installation, so pullRelayPending can later scope its drain to that same enrollment instead of + * "any enrollment secret valid for this installation" — closing the hole where a stale re-enrolled container + * could drain and destructively-ack a live one's queue. Optional so config_push (installation-wide, not + * consumer-specific) can keep enqueueing with no enrollment context at all. */ export async function enqueueRelayPending( env: Env, - args: { deliveryId: string; installationId: number; eventName: string; rawBody: string }, + args: { deliveryId: string; installationId: number; eventName: string; rawBody: string; enrollId?: string | null }, ): Promise { await pruneRelayPending(env); const coalesceKey = relayPendingCoalesceKey(args.eventName, args.rawBody); const inserted = await env.DB .prepare( - "INSERT INTO orb_relay_pending (delivery_id, installation_id, event_name, raw_body, coalesce_key) VALUES (?, ?, ?, ?, ?) ON CONFLICT(delivery_id) DO NOTHING", + "INSERT INTO orb_relay_pending (delivery_id, installation_id, event_name, raw_body, coalesce_key, enroll_id) VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT(delivery_id) DO NOTHING", ) - .bind(args.deliveryId, args.installationId, args.eventName, args.rawBody, coalesceKey) + .bind(args.deliveryId, args.installationId, args.eventName, args.rawBody, coalesceKey, args.enrollId ?? null) .run(); if (coalesceKey && inserted.meta.changes > 0) { await env.DB @@ -418,21 +425,31 @@ function relayPendingCoalesceKey(eventName: string, rawBody: string): string | n /** Drain pending pull-mode events for an installation. The engine calls this outbound (it can't be pushed to): * 1) prune TTL-expired rows fleet-wide, 2) delete rows the caller ACKs (scoped to this install so one container * can never ack another's), then 3) return the next ordered batch. `ack` and `limit` are both capped at the batch - * size to bound the SQL and the response. */ + * size to bound the SQL and the response. + * + * #9150: `enrollId`, when supplied, scopes BOTH the ack-delete and the select to rows tagged for that specific + * enrollment (or untagged legacy/config_push rows) — not merely `installation_id`. Without it, any enrollment + * secret valid for the installation could drain (and destructively-ack) another live enrollment's queue, e.g. + * a stale re-enrolled container still holding a valid-but-superseded secret. Optional (and defaults to + * matching only untagged rows when omitted) so a caller that hasn't threaded an enrollId through yet degrades + * to the narrower, safer set rather than erroring. */ export async function pullRelayPending( env: Env, installationId: number, - opts?: { ack?: string[] | undefined; limit?: number | undefined }, + opts?: { ack?: string[] | undefined; limit?: number | undefined; enrollId?: string | null | undefined }, ): Promise { // Prune rows the engine never came back for (same datetime() comparison style as retryFailedRelays). await pruneRelayPending(env); + const enrollId = opts?.enrollId ?? null; const ack = opts?.ack?.slice(0, RELAY_PENDING_BATCH_SIZE) ?? []; if (ack.length) { const placeholders = ack.map(() => "?").join(", "); await env.DB - .prepare(`DELETE FROM orb_relay_pending WHERE installation_id = ? AND delivery_id IN (${placeholders})`) - .bind(installationId, ...ack) + .prepare( + `DELETE FROM orb_relay_pending WHERE installation_id = ? AND (enroll_id = ? OR enroll_id IS NULL) AND delivery_id IN (${placeholders})`, + ) + .bind(installationId, enrollId, ...ack) .run(); } @@ -445,8 +462,10 @@ export async function pullRelayPending( : RELAY_PENDING_BATCH_SIZE; const limit = Math.min(requestedLimit, RELAY_PENDING_BATCH_SIZE); const { results } = await env.DB - .prepare("SELECT delivery_id, event_name, raw_body, kind FROM orb_relay_pending WHERE installation_id = ? ORDER BY created_at, delivery_id LIMIT ?") - .bind(installationId, limit) + .prepare( + "SELECT delivery_id, event_name, raw_body, kind FROM orb_relay_pending WHERE installation_id = ? AND (enroll_id = ? OR enroll_id IS NULL) ORDER BY created_at, delivery_id LIMIT ?", + ) + .bind(installationId, enrollId, limit) .all<{ delivery_id: string; event_name: string; raw_body: string; kind: string }>(); return results.map((r) => ({ deliveryId: r.delivery_id, eventName: r.event_name, rawBody: r.raw_body, kind: r.kind })); } @@ -536,16 +555,26 @@ export async function forwardOrbEvent( // then the newest relay registration, then the newest enrollment. The final tie-break is the implicit rowid // (monotonic insertion order) — enroll_id is a random opaque token, and CURRENT_TIMESTAMP ties at second // resolution, so rowid is the only stable "most recently inserted" key when those collide (#1783). - const row = await env.DB + // + // #9150: fetch every LIVE row (not just the winner via .first()) so a second, briefly-overlapping live + // enrollment for the same installation (a blue/green swap, or a secret rotated but not yet revoked) is + // OBSERVABLE — the sibling revoke-on-reissue fix (#9149) narrows the window this can happen in, but a + // rollout swap can still produce two live consumers for a short time, and that should never be silent. + const { results: liveRows } = await env.DB .prepare( - "SELECT relay_mode, relay_url, relay_secret_enc, relay_secret_iv, relay_secret_salt FROM orb_enrollments WHERE installation_id = ? AND state = 'enrolled' AND revoked_at IS NULL ORDER BY (relay_registered_at IS NOT NULL) DESC, relay_registered_at DESC, enrolled_at DESC, rowid DESC", + "SELECT enroll_id, relay_mode, relay_url, relay_secret_enc, relay_secret_iv, relay_secret_salt FROM orb_enrollments WHERE installation_id = ? AND state = 'enrolled' AND revoked_at IS NULL ORDER BY (relay_registered_at IS NOT NULL) DESC, relay_registered_at DESC, enrolled_at DESC, rowid DESC", ) .bind(args.installationId) - .first<{ relay_mode: string; relay_url: string | null; relay_secret_enc: string | null; relay_secret_iv: string | null; relay_secret_salt: string | null }>(); + .all<{ enroll_id: string; relay_mode: string; relay_url: string | null; relay_secret_enc: string | null; relay_secret_iv: string | null; relay_secret_salt: string | null }>(); + const row = liveRows[0]; if (!row) return "ignored"; // not a brokered self-host (or revoked) — nothing to relay to + if (liveRows.length > 1) incr("loopover_orb_relay_multiple_live_enrollments_total"); // Pull mode (#16): a tailnet container can't be pushed to, so ENQUEUE the event for it to drain outbound. + // Tagged with the WINNING enrollment's id (#9150) so pullRelayPending can later scope its drain to exactly + // this enrollment instead of "any secret valid for this installation" — closing the hole where a second, + // stale-but-still-valid enrollment could drain and destructively-ack this event first. if (row.relay_mode === "pull") { - await enqueueRelayPending(env, { deliveryId: args.deliveryId, installationId: args.installationId, eventName: args.eventName, rawBody: args.rawBody }); + await enqueueRelayPending(env, { deliveryId: args.deliveryId, installationId: args.installationId, eventName: args.eventName, rawBody: args.rawBody, enrollId: row.enroll_id }); return "queued"; } // Push mode with nothing registered (relay_url null) or no decryption key → skip. relay_secret_enc/iv are written diff --git a/src/queue/job-dispatch.ts b/src/queue/job-dispatch.ts index 706325e6cc..5185bff16f 100644 --- a/src/queue/job-dispatch.ts +++ b/src/queue/job-dispatch.ts @@ -37,6 +37,9 @@ import { runSelfTuneBreaker } from "../review/outcomes-wire"; import { isRagEnabled } from "../review/rag-wire"; import { processSubmitDraft } from "../services/draft"; import { retryFailedRelays } from "../orb/relay"; +import { resolveFederatedIntelligenceManifestOverride, runFederatedPeerSyncTick } from "../orb/federated-benchmark"; +import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; +import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest"; import { loadPendingAprRepoTransfers, pollPendingAprRepoTransfers, @@ -500,6 +503,20 @@ export async function processJob(env: Env, message: JobMessage): Promise { // an empty table). Never throws. await retryFailedRelays(env); return; + case "federated-peer-sync": { + // Federated peer-sync tick (#9148/#9166): pulls + trust-gates + persists peer bundles into the + // maintainer-dashboard benchmark cache, and pushes this instance's own bundle to its configured + // collector. Defense-in-depth (same pattern as ops-alerts above): the cron only enqueues this when + // federatedIntelligence.enabled is true, but a stale in-flight job that lands after the operator flips + // it off must still no-op. The cheap override check runs first; the FULL manifest (peerKeys, + // collectorUrl/collectorMode — fields the lightweight override doesn't carry) is only loaded once armed. + const federatedOverride = await resolveFederatedIntelligenceManifestOverride(env); + if (federatedOverride.enabled) { + const federatedManifest = await loadRepoFocusManifest(env, resolveLoopOverSelfRepoFullName(env)); + await runFederatedPeerSyncTick(federatedManifest, env.DB); + } + return; + } /* v8 ignore start -- live-loop wiring: binds the injectable, unit-tested pollPendingAprRepoTransfers (#7741) to its real dependencies. The detection/expiry/pause logic is covered directly in test/unit/orb-apr-repo-transfer.test.ts; this arm is a no-op today (loadPendingAprRepoTransfers fail-empties diff --git a/src/selfhost/orb-collector.ts b/src/selfhost/orb-collector.ts index 516b84731f..17846633f4 100644 --- a/src/selfhost/orb-collector.ts +++ b/src/selfhost/orb-collector.ts @@ -102,44 +102,69 @@ async function loadReuseCounters(db: D1Database, nowMs: number): Promise { +/** Read-or-generate-and-persist a single-purpose, per-instance 256-bit secret under `flagKey` in system_flags. + * Shared by every dedicated secret this file mints (the anonymization secret, the instance identity secret) so + * each stays its own independent value — never derived from, or reused as, another (#9147's key-separation + * fix). Race-safe across instances sharing a Postgres DB: OR IGNORE keeps the first writer's value; the + * re-read returns whichever value won, so every instance converges on the same secret. */ +async function getOrCreateSystemSecret(db: D1Database, flagKey: string, seed?: string): Promise { const read = async (): Promise => { - const row = await db - .prepare(`SELECT value FROM system_flags WHERE key = ?`) - .bind(ANON_SECRET_FLAG) - .first<{ value: string }>(); + const row = await db.prepare(`SELECT value FROM system_flags WHERE key = ?`).bind(flagKey).first<{ value: string }>(); return row?.value; }; const existing = await read(); if (existing) return existing; - const generated = generateAnonSecret(); // 256-bit, 64 hex chars - // Race-safe across instances sharing a Postgres DB: OR IGNORE keeps the first writer's key; the re-read - // returns whichever value won, so every instance converges on the same secret. + const generated = seed ?? generateAnonSecret(); // 256-bit, 64 hex chars await db .prepare(`INSERT OR IGNORE INTO system_flags (key, value, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP)`) - .bind(ANON_SECRET_FLAG, generated) + .bind(flagKey, generated) .run(); /* v8 ignore next -- a row always exists after INSERT OR IGNORE, so the ?? fallback is unreachable */ return (await read()) ?? generated; } +/** + * The instance's DEDICATED anonymization secret: a 256-bit random key generated once and persisted in + * system_flags, then reused on every export. Stable across restarts so a repo/PR always hashes the same + * way (the collector can dedup), per-instance, and SINGLE-PURPOSE — never the App private key, the + * webhook-verification secret, or (since #9147) the federated bundle signing key (key separation). The + * collector never holds it, so it cannot de-anonymize. + */ +export async function getOrCreateAnonSecret(db: D1Database): Promise { + return getOrCreateSystemSecret(db, ANON_SECRET_FLAG); +} + +/** system_flags key for the instance identity secret (#9147) — deliberately distinct from ANON_SECRET_FLAG. */ +const INSTANCE_IDENTITY_SECRET_FLAG = "orb:instance_identity_secret"; + +/** + * The instance's DEDICATED identity secret (#9147): what `instanceId` hashes for a brokered instance with no + * App id, kept in its OWN system_flags row so rotating the anonymization secret can never change a brokered + * instance's identity (and therefore never resets the export watermark or the federated peer-median's + * per-instance accounting keyed on it). On FIRST read (no row yet — every instance that upgraded from before + * this secret existed, or a never-yet-exported instance), it seeds from the CURRENT anon secret: an + * already-live instance keeps the exact instanceId it already has (no disruptive identity change on deploy), + * and a brand-new instance's anon secret is just as freshly generated, so reusing it here costs it nothing. + * From that point on the two secrets are independent rows and can be rotated separately. + */ +export async function getOrCreateInstanceIdentitySecret(db: D1Database): Promise { + const anonSecret = await getOrCreateAnonSecret(db); + return getOrCreateSystemSecret(db, INSTANCE_IDENTITY_SECRET_FLAG, anonSecret); +} + /** Map the gate's free-text reasonCode to a fixed, low-cardinality category — done at the source so the raw * (possibly repo-specific) reason string never leaves the instance. */ export function bucketReasonCode(summary: string | null | undefined): string { @@ -233,7 +258,9 @@ export async function exportOrbBatch(db: D1Database, batchSize = 200, fetchFn: t const collectorUrl = process.env.ORB_COLLECTOR_URL ?? "https://api.loopover.ai/v1/orb/ingest"; const secret = await getOrCreateAnonSecret(db); const anonymize = (process.env.ORB_ANONYMIZE ?? "true").toLowerCase() !== "false"; - const instance = instanceId(secret); + // #9147: instanceId is derived from the DEDICATED identity secret, never the anon secret above — so + // rotating ORB_ANONYMIZE's key can never change this instance's identity or reset its export watermark. + const instance = instanceId(await getOrCreateInstanceIdentitySecret(db)); // Read this instance's export watermark (resumes where the last run left off). const cursorRow = await db diff --git a/src/types.ts b/src/types.ts index 9cfb94b37f..8a6a6b6c4f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -299,6 +299,14 @@ export type JobMessage = type: "retry-orb-relay"; requestedBy: "schedule" | "test"; } + | { + // Federated peer-sync tick (#9148/#9166): pulls + trust-gates + persists peer bundles into the + // maintainer-dashboard benchmark cache, AND pushes this instance's own bundle to its configured + // collector. Moves that work off the dashboard's own request path onto a background cadence. Enqueued + // by the cron ONLY when the loopover self-repo's `.loopover.yml federatedIntelligence.enabled` is true. + type: "federated-peer-sync"; + requestedBy: "schedule" | "test"; + } | { // APR repo-transfer acceptance/expiry detection (#7741): resolve every pending APR transfer — probe GitHub, // mark accepted / accepted-and-departed / expired (>7 days), reconcile the per-repo AMS pause. Enqueued by diff --git a/test/integration/orb-broker.test.ts b/test/integration/orb-broker.test.ts index f01c4f4333..7e60de2561 100644 --- a/test/integration/orb-broker.test.ts +++ b/test/integration/orb-broker.test.ts @@ -3,12 +3,14 @@ import { createApp } from "../../src/api/routes"; import { hashToken } from "../../src/auth/security"; import { brokerOrbToken, + countLiveEnrollmentsForInstallation, isOrbBrokerEnabled, issueOrbEnrollment, issueOrbStoredSecret, ORB_SECRET_TYPE_AMS_GITHUB_TOKEN, ORB_SECRET_TYPE_GITHUB_TOKEN, ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL, + revokeAllLiveEnrollmentsForInstallation, revokeOrbEnrollment, } from "../../src/orb/broker"; import { MAX_ORB_RELAY_REGISTER_BODY_BYTES } from "../../src/orb/relay"; @@ -88,6 +90,80 @@ describe("issueOrbEnrollment", () => { const row = await db(e).prepare("SELECT secret_type, installation_id FROM orb_enrollments WHERE installation_id=203").first<{ secret_type: string; installation_id: number }>(); expect(row).toMatchObject({ secret_type: ORB_SECRET_TYPE_AMS_GITHUB_TOKEN, installation_id: 203 }); }); + + it("#9149: re-enrolling WITHOUT rotate mints an additional secret, leaving the prior one live (blue/green default)", async () => { + const e = await brokerEnv(); + await seedInstall(e, 210, { registered: 1 }); + const first = (await issueOrbEnrollment(e, 210)) as { enrollId: string; secret: string }; + const second = (await issueOrbEnrollment(e, 210)) as { enrollId: string; secret: string }; + expect(first.enrollId).not.toBe(second.enrollId); + expect(await countLiveEnrollmentsForInstallation(e, 210)).toBe(2); + const firstRow = await db(e).prepare("SELECT state FROM orb_enrollments WHERE enroll_id = ?").bind(first.enrollId).first<{ state: string }>(); + expect(firstRow?.state).toBe("enrolled"); // untouched — the leaked-secret bug this closes is the OPPOSITE default + }); + + it("#9149: re-enrolling WITH rotate:true revokes every prior live enrollment before minting the new one", async () => { + const e = await brokerEnv(); + await seedInstall(e, 211, { registered: 1 }); + const first = (await issueOrbEnrollment(e, 211)) as { enrollId: string; secret: string }; + const second = (await issueOrbEnrollment(e, 211, undefined, ORB_SECRET_TYPE_GITHUB_TOKEN, { rotate: true })) as { enrollId: string; secret: string }; + expect(await countLiveEnrollmentsForInstallation(e, 211)).toBe(1); + const firstRow = await db(e).prepare("SELECT state, revoked_at FROM orb_enrollments WHERE enroll_id = ?").bind(first.enrollId).first<{ state: string; revoked_at: string | null }>(); + expect(firstRow).toMatchObject({ state: "revoked" }); + expect(firstRow?.revoked_at).not.toBeNull(); + expect(await brokerOrbToken(e, first.secret)).toEqual({ error: "invalid_enrollment" }); // the leaked/superseded secret no longer mints + const secondRow = await db(e).prepare("SELECT state FROM orb_enrollments WHERE enroll_id = ?").bind(second.enrollId).first<{ state: string }>(); + expect(secondRow?.state).toBe("enrolled"); + }); + + it("#9149: rotate on a FIRST enrollment (nothing prior to revoke) is a no-op revoke, not an error", async () => { + const e = await brokerEnv(); + await seedInstall(e, 212, { registered: 1 }); + const issued = await issueOrbEnrollment(e, 212, undefined, ORB_SECRET_TYPE_GITHUB_TOKEN, { rotate: true }); + expect(issued).toMatchObject({ enrollId: expect.stringMatching(/^orbenr_/) }); + expect(await countLiveEnrollmentsForInstallation(e, 212)).toBe(1); + }); +}); + +describe("revokeAllLiveEnrollmentsForInstallation + countLiveEnrollmentsForInstallation (#9149)", () => { + it("counts 0 for an installation with no enrollments at all", async () => { + const e = await brokerEnv(); + expect(await countLiveEnrollmentsForInstallation(e, 9999)).toBe(0); + }); + + it("revokes every live enrollment for an installation and returns the count, leaving OTHER installations untouched", async () => { + const e = await brokerEnv(); + await seedInstall(e, 220, { registered: 1 }); + await seedInstall(e, 221, { registered: 1 }); + const a = (await issueOrbEnrollment(e, 220)) as { enrollId: string }; + const b = (await issueOrbEnrollment(e, 220)) as { enrollId: string }; + await issueOrbEnrollment(e, 221); + + expect(await revokeAllLiveEnrollmentsForInstallation(e, 220)).toBe(2); + expect(await countLiveEnrollmentsForInstallation(e, 220)).toBe(0); + const [rowA, rowB] = await Promise.all([ + db(e).prepare("SELECT state FROM orb_enrollments WHERE enroll_id = ?").bind(a.enrollId).first<{ state: string }>(), + db(e).prepare("SELECT state FROM orb_enrollments WHERE enroll_id = ?").bind(b.enrollId).first<{ state: string }>(), + ]); + expect(rowA?.state).toBe("revoked"); + expect(rowB?.state).toBe("revoked"); + // A different installation's enrollment is untouched by another installation's revoke-all. + expect(await countLiveEnrollmentsForInstallation(e, 221)).toBe(1); + }); + + it("revoking an installation with no live enrollments returns 0, not an error", async () => { + const e = await brokerEnv(); + await seedInstall(e, 222, { registered: 1 }); + expect(await revokeAllLiveEnrollmentsForInstallation(e, 222)).toBe(0); + }); + + it("works on a SUSPENDED/disabled installation — revoking access is never blocked by unrelated admin state", async () => { + const e = await brokerEnv(); + await seedInstall(e, 223, { registered: 1, suspended_at: "2026-01-01T00:00:00Z", self_enrollment_disabled: 1 }); + const enrolled = (await issueOrbEnrollment(e, 223)) as { enrollId: string }; + expect(enrolled).toMatchObject({ enrollId: expect.stringMatching(/^orbenr_/) }); + expect(await revokeAllLiveEnrollmentsForInstallation(e, 223)).toBe(1); + }); }); describe("issueOrbStoredSecret", () => { @@ -438,6 +514,22 @@ describe("broker endpoints", () => { expect(await tokRes.json()).toMatchObject({ token: "ghs_flow", installationId: 400 }); }); + it("#9149: POST /v1/internal/orb/enrollments defaults to append (no rotate); { rotate: true } revokes prior live enrollments first", async () => { + const e = await brokerEnv(); + await seedInstall(e, 401, { registered: 1 }); + const first = (await (await app.request("/v1/internal/orb/enrollments", { method: "POST", headers: auth, body: JSON.stringify({ installationId: 401 }) }, e)).json()) as { secret: string }; + // Default (no rotate field at all) — append, matching every existing caller's unaffected behavior. + await app.request("/v1/internal/orb/enrollments", { method: "POST", headers: auth, body: JSON.stringify({ installationId: 401 }) }, e); + expect(await db(e).prepare("SELECT COUNT(*) AS n FROM orb_enrollments WHERE installation_id=401 AND state='enrolled'").first<{ n: number }>()).toMatchObject({ n: 2 }); + tokenFetch("ghs_before_rotate"); + expect((await app.request("/v1/orb/token", { method: "POST", headers: { authorization: `Bearer ${first.secret}` } }, e)).status).toBe(200); // first secret still valid + + // rotate: true — revokes both prior live rows, mints exactly one fresh one. + await app.request("/v1/internal/orb/enrollments", { method: "POST", headers: auth, body: JSON.stringify({ installationId: 401, rotate: true }) }, e); + expect(await db(e).prepare("SELECT COUNT(*) AS n FROM orb_enrollments WHERE installation_id=401 AND state='enrolled'").first<{ n: number }>()).toMatchObject({ n: 1 }); + expect((await app.request("/v1/orb/token", { method: "POST", headers: { authorization: `Bearer ${first.secret}` } }, e)).status).toBe(401); // superseded + }); + it("/v1/orb/token: 401 without a Bearer secret, 401 on a bad secret, 403 when the install became ineligible", async () => { const e = await brokerEnv(); expect((await app.request("/v1/orb/token", { method: "POST" }, e)).status).toBe(401); diff --git a/test/integration/orb-oauth.test.ts b/test/integration/orb-oauth.test.ts index 1c1650c9e3..b4a30a36d9 100644 --- a/test/integration/orb-oauth.test.ts +++ b/test/integration/orb-oauth.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createApp } from "../../src/api/routes"; import * as githubClientModule from "../../src/github/client"; import { exchangeOrbOAuthCode, fetchOrbOAuthUser, verifyInstallationAdmin } from "../../src/orb/oauth"; +import { issueOrbEnrollment } from "../../src/orb/broker"; import { createTestEnv, type TestD1Database } from "../helpers/d1"; const asFetch = (fn: (url: string) => Promise): typeof fetch => ((url: RequestInfo | URL) => fn(String(url))) as typeof fetch; @@ -127,13 +128,103 @@ describe("maintainer self-enrollment via the OAuth callback", () => { stubGitHub(); const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=500", {}, e); expect(res.status).toBe(200); + expect(res.headers.get("cache-control")).toBe("no-store"); // #9149: a plaintext secret must never be cacheable const html = await res.text(); expect(html).toContain("Your enrollment secret"); expect(html).toMatch(/orbsec_/); + expect(html).toContain("1 live enrollment secret,"); // #9149: live-enrollment count surfaced, singular form const row = await db(e).prepare("SELECT maintainer_login, maintainer_github_id FROM orb_enrollments WHERE installation_id=500").first<{ maintainer_login: string; maintainer_github_id: number }>(); expect(row).toMatchObject({ maintainer_login: "alice", maintainer_github_id: 7 }); }); + it("#9149: the secret page's live-enrollment count reflects an earlier enrollment, and pluralizes correctly", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 510, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + stubGitHub(); + await app.request("/v1/orb/oauth/callback?code=abc1&installation_id=510", {}, e); + const res = await app.request("/v1/orb/oauth/callback?code=abc2&installation_id=510", {}, e); + const html = await res.text(); + expect(html).toContain("2 live enrollment secrets,"); // plural + expect(html).toMatch(/state=510%3Arotate/); // the Rotate link carries installation 510 + expect(html).toMatch(/state=510%3Arevoke/); // the Revoke-all link carries installation 510 + }); + + it("#9149: state=:rotate revokes every PRIOR live enrollment and mints exactly one fresh secret", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 511, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + stubGitHub(); + await app.request("/v1/orb/oauth/callback?code=abc1&installation_id=511", {}, e); // first, ordinary enrollment + const rotateRes = await app.request("/v1/orb/oauth/callback?code=abc2&state=511:rotate", {}, e); // no installation_id query param — carried in state + expect(rotateRes.status).toBe(200); + const html = await rotateRes.text(); + expect(html).toContain("Your enrollment secret"); + expect(html).toContain("1 live enrollment secret,"); // back down to one after rotating + const rows = await db(e).prepare("SELECT state FROM orb_enrollments WHERE installation_id=511 ORDER BY rowid").all<{ state: string }>(); + expect(rows.results.map((r) => r.state)).toEqual(["revoked", "enrolled"]); + }); + + it("#9149: state=:revoke revokes every live enrollment and mints nothing new", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 512, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + stubGitHub(); + await app.request("/v1/orb/oauth/callback?code=abc1&installation_id=512", {}, e); + await app.request("/v1/orb/oauth/callback?code=abc2&installation_id=512", {}, e); + const revokeRes = await app.request("/v1/orb/oauth/callback?code=abc3&state=512:revoke", {}, e); + expect(revokeRes.status).toBe(200); + const html = await revokeRes.text(); + expect(html).toContain("Enrollment secrets revoked"); + expect(html).toContain("2 live enrollment secrets have been revoked"); + expect(await db(e).prepare("SELECT COUNT(*) AS n FROM orb_enrollments WHERE installation_id=512 AND state='enrolled'").first<{ n: number }>()).toMatchObject({ n: 0 }); + // No new row was minted by a revoke request. + expect(await db(e).prepare("SELECT COUNT(*) AS n FROM orb_enrollments WHERE installation_id=512").first<{ n: number }>()).toMatchObject({ n: 2 }); + }); + + it("#9149: a REVOKE request still requires admin-of-installation — a non-admin revokes nothing", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 513, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + await issueOrbEnrollment(e, 513); + stubGitHub({ membership: { role: "member", state: "active", organization: { id: 20 } } }); + const res = await app.request("/v1/orb/oauth/callback?code=abc&state=513:revoke", {}, e); + expect(res.status).toBe(403); + expect(await db(e).prepare("SELECT COUNT(*) AS n FROM orb_enrollments WHERE installation_id=513 AND state='enrolled'").first<{ n: number }>()).toMatchObject({ n: 1 }); + }); + + it("#9149: revoke works EVEN ON a suspended/self-enrollment-disabled installation — unrelated admin state never blocks it", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 514, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1, suspended_at: "2026-01-01T00:00:00Z", self_enrollment_disabled: 1 }); + await issueOrbEnrollment(e, 514); + stubGitHub(); + const res = await app.request("/v1/orb/oauth/callback?code=abc&state=514:revoke", {}, e); + expect(res.status).toBe(200); + expect(await res.text()).toContain("1 live enrollment secret has been revoked"); + }); + + it("#9149: revoking an installation with no live enrollments still succeeds, reporting 0", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 515, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + stubGitHub(); + const res = await app.request("/v1/orb/oauth/callback?code=abc&state=515:revoke", {}, e); + expect(res.status).toBe(200); + expect(await res.text()).toContain("0 live enrollment secrets have been revoked"); + }); + + it("#9149: an unrecognized state action falls back to a plain enrollment (defensive default)", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 516, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + stubGitHub(); + const res = await app.request("/v1/orb/oauth/callback?code=abc&state=516:bogus", {}, e); + expect(res.status).toBe(200); + expect(await res.text()).toContain("Your enrollment secret"); + }); + + it("#9149: a malformed state (non-numeric id, no installation_id query either) falls through to the generic landing page", async () => { + const e = brokeredEnv(); + stubGitHub(); + const res = await app.request("/v1/orb/oauth/callback?code=abc&state=not-a-number:rotate", {}, e); + expect(await res.text()).toContain("LoopOver Orb connected"); + }); + + it("a NON-admin is refused (403), NO enrollment created, and the install is NOT auto-registered — the escalation gate", async () => { const e = brokeredEnv(); await seedInstall(e, { installation_id: 501, account_login: "acme", account_type: "Organization", account_id: 20, registered: 0 }); diff --git a/test/integration/orb-onboarding.test.ts b/test/integration/orb-onboarding.test.ts index 5703a78274..e43c31845c 100644 --- a/test/integration/orb-onboarding.test.ts +++ b/test/integration/orb-onboarding.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createApp } from "../../src/api/routes"; +import { issueOrbEnrollment, revokeAllLiveEnrollmentsForInstallation } from "../../src/orb/broker"; import { createTestEnv, type TestD1Database } from "../helpers/d1"; async function pkcs8Pem(): Promise { @@ -34,6 +35,30 @@ describe("Central Orb installation registry routes (/v1/internal/orb/installatio expect((await app.request("/v1/internal/orb/installations", {}, createTestEnv())).status).toBe(401); }); + it("#9149: surfaces liveEnrollmentCount per installation, reflecting a revoke and untouched by ANOTHER installation's enrollments", async () => { + const env = createTestEnv({ ORB_BROKER_ENABLED: "true" }); + await seed(env, 102, 1); + await seed(env, 103, 1); + await issueOrbEnrollment(env, 102); + await issueOrbEnrollment(env, 102); + await issueOrbEnrollment(env, 103); + const before = (await (await app.request("/v1/internal/orb/installations", { headers: auth }, env)).json()) as { installations: Array<{ installationId: number; liveEnrollmentCount: number }> }; + expect(before.installations.find((i) => i.installationId === 102)?.liveEnrollmentCount).toBe(2); + expect(before.installations.find((i) => i.installationId === 103)?.liveEnrollmentCount).toBe(1); + + await revokeAllLiveEnrollmentsForInstallation(env, 102); + const after = (await (await app.request("/v1/internal/orb/installations", { headers: auth }, env)).json()) as { installations: Array<{ installationId: number; liveEnrollmentCount: number }> }; + expect(after.installations.find((i) => i.installationId === 102)?.liveEnrollmentCount).toBe(0); + expect(after.installations.find((i) => i.installationId === 103)?.liveEnrollmentCount).toBe(1); // unaffected by the OTHER install's revoke + }); + + it("reports liveEnrollmentCount 0 for an installation with no enrollments at all", async () => { + const env = createTestEnv(); + await seed(env, 104); + const res = (await (await app.request("/v1/internal/orb/installations", { headers: auth }, env)).json()) as { installations: Array<{ installationId: number; liveEnrollmentCount: number }> }; + expect(res.installations.find((i) => i.installationId === 104)?.liveEnrollmentCount).toBe(0); + }); + it("registers an installation, then unregisters it", async () => { const env = createTestEnv(); await seed(env, 101); diff --git a/test/integration/orb-relay.test.ts b/test/integration/orb-relay.test.ts index f7e1c0d02f..b9e6598d75 100644 --- a/test/integration/orb-relay.test.ts +++ b/test/integration/orb-relay.test.ts @@ -3,6 +3,7 @@ import { createApp } from "../../src/api/routes"; import { issueOrbEnrollment } from "../../src/orb/broker"; import { relayForward } from "../../src/orb/webhook"; import { enqueueRelayPending, forwardOrbEvent, MAX_ORB_RELAY_REGISTER_BODY_BYTES, pullRelayPending, readOrbRelayRegisterBody, registerOrbRelay, relaySignature, relayVerify, retryFailedRelays, storeRelayFailure } from "../../src/orb/relay"; +import { counterValue, resetMetrics } from "../../src/selfhost/metrics"; import { createTestEnv, type TestD1Database } from "../helpers/d1"; const db = (e: Env) => e.DB as unknown as TestD1Database; @@ -274,6 +275,61 @@ describe("forwardOrbEvent", () => { expect(h["x-orb-signature-256"]).toBe(`sha256=${await relaySignature(freshSecret, body)}`); }); + it("#9150: PULL mode tags each enqueued row with the WINNING enrollment's id, and emits a metric when a second live enrollment exists", async () => { + const e = brokeredEnv(); + await seedInstall(e, 806); + resetMetrics(); + const secretA = ((await issueOrbEnrollment(e, 806)) as { secret: string }).secret; + await registerOrbRelay(e, secretA, "https://a.example/v1/orb/relay"); + await db(e).prepare("UPDATE orb_enrollments SET relay_mode = 'pull' WHERE installation_id = 806").run(); + // Only ONE live enrollment so far — the metric must not fire yet. + expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 806, deliveryId: "single-enroll", rawBody: "{}" })).toBe("queued"); + expect(counterValue("loopover_orb_relay_multiple_live_enrollments_total")).toBe(0); + const rowA = await db(e).prepare("SELECT enroll_id FROM orb_enrollments WHERE installation_id = 806 ORDER BY rowid").first<{ enroll_id: string }>(); + const taggedSingle = await db(e).prepare("SELECT enroll_id FROM orb_relay_pending WHERE delivery_id = 'single-enroll'").first<{ enroll_id: string | null }>(); + expect(taggedSingle?.enroll_id).toBe(rowA?.enroll_id); + + // A second issuance (blue/green swap, or a rotation that hasn't revoked the old secret yet) leaves TWO live rows. + const secretB = ((await issueOrbEnrollment(e, 806)) as { secret: string }).secret; + await registerOrbRelay(e, secretB, "https://b.example/v1/orb/relay"); + await db(e).prepare("UPDATE orb_enrollments SET relay_mode = 'pull' WHERE installation_id = 806").run(); + const rowB = await db(e).prepare("SELECT enroll_id FROM orb_enrollments WHERE installation_id = 806 ORDER BY rowid DESC").first<{ enroll_id: string }>(); + expect(await forwardOrbEvent(e, { eventName: "pull_request", installationId: 806, deliveryId: "double-enroll", rawBody: "{}" })).toBe("queued"); + expect(counterValue("loopover_orb_relay_multiple_live_enrollments_total")).toBe(1); // now observable, not silent + const taggedDouble = await db(e).prepare("SELECT enroll_id FROM orb_relay_pending WHERE delivery_id = 'double-enroll'").first<{ enroll_id: string | null }>(); + expect(taggedDouble?.enroll_id).toBe(rowB?.enroll_id); // tagged for the newest (winning) enrollment, same election as push + + // pullRelayPending scoped to enrollment A must see only its own event (+ any untagged legacy rows) — NOT + // enrollment B's, even though both belong to the same installation. This is the actual #9150 fix: before it, + // "any valid secret for the installation" could drain and destructively-ack the other consumer's queue. + const drainedByA = await pullRelayPending(e, 806, { enrollId: rowA?.enroll_id ?? null }); + expect(drainedByA.map((ev) => ev.deliveryId)).toEqual(["single-enroll"]); + const drainedByB = await pullRelayPending(e, 806, { enrollId: rowB?.enroll_id ?? null }); + expect(drainedByB.map((ev) => ev.deliveryId)).toEqual(["double-enroll"]); + }); + + it("#9150: an enrollment scope ALSO drains untagged legacy rows (enroll_id IS NULL never orphaned)", async () => { + const e = brokeredEnv(); + await enqueueRelayPending(e, { deliveryId: "legacy-untagged", installationId: 807, eventName: "pull_request", rawBody: "{}" }); + await enqueueRelayPending(e, { deliveryId: "tagged-for-x", installationId: 807, eventName: "pull_request", rawBody: '{"n":1}', enrollId: "enroll-x" }); + const drained = await pullRelayPending(e, 807, { enrollId: "enroll-x" }); + expect(drained.map((ev) => ev.deliveryId).sort()).toEqual(["legacy-untagged", "tagged-for-x"]); + // A DIFFERENT enrollment for the same install sees the untagged row too, but not the one tagged for "enroll-x". + const drainedByOther = await pullRelayPending(e, 807, { enrollId: "enroll-y" }); + expect(drainedByOther.map((ev) => ev.deliveryId)).toEqual(["legacy-untagged"]); + }); + + it("#9150: ack scoped to an enrollment cannot delete another live enrollment's row for the same installation", async () => { + const e = brokeredEnv(); + await enqueueRelayPending(e, { deliveryId: "owned-by-x", installationId: 808, eventName: "pull_request", rawBody: "{}", enrollId: "enroll-x" }); + await enqueueRelayPending(e, { deliveryId: "owned-by-y", installationId: 808, eventName: "pull_request", rawBody: '{"n":1}', enrollId: "enroll-y" }); + // enroll-x tries to ack BOTH its own row and enroll-y's — only its own is removed. + const remaining = await pullRelayPending(e, 808, { enrollId: "enroll-x", ack: ["owned-by-x", "owned-by-y"] }); + expect(remaining).toEqual([]); // enroll-x's own scoped view has nothing left to return + const victim = await db(e).prepare("SELECT delivery_id FROM orb_relay_pending WHERE delivery_id = 'owned-by-y'").first(); + expect(victim ?? null).not.toBeNull(); // enroll-y's row survived enroll-x's ack — the actual #9150 fix + }); + it("returns FAILED (never throws) on a non-ok response or a thrown fetch — the Orb 202 always stands", async () => { const e = brokeredEnv(); const secret = await enroll(e, 802); diff --git a/test/unit/federated-benchmark.test.ts b/test/unit/federated-benchmark.test.ts index 68d822c1fe..a59d9a26d1 100644 --- a/test/unit/federated-benchmark.test.ts +++ b/test/unit/federated-benchmark.test.ts @@ -3,7 +3,7 @@ import { createHmac } from "node:crypto"; import { describe, expect, it, vi } from "vitest"; import { createD1Adapter, nodeSqliteDriver } from "../../src/selfhost/d1-adapter"; import { canonicalizeFederatedBundleBody, FEDERATED_BUNDLE_SCHEMA_VERSION, type FederatedSignalBundle } from "../../src/orb/federated-bundle"; -import { buildFederatedBenchmark } from "../../src/orb/federated-benchmark"; +import { buildFederatedBenchmark, refreshFederatedBenchmarkCache, runFederatedPeerSyncTick } from "../../src/orb/federated-benchmark"; import type { FederatedCollectorMode, FocusManifest } from "../../src/signals/focus-manifest"; const URL_OK = "https://collector.example.org/v1/federated"; @@ -13,7 +13,8 @@ const PEER_KEY_B = "b".repeat(64); const UNTRUSTED_KEY = "c".repeat(64); const NOW = Date.parse("2026-07-16T00:00:00Z"); -/** In-memory DB with the tables buildFederatedBundle reads (mirrors federated-bundle.test.ts's makeDb). */ +/** In-memory DB with the tables buildFederatedBundle/the peer-state + benchmark caches read (mirrors + * federated-bundle.test.ts's makeDb). */ function makeDb(): D1Database { const driver = nodeSqliteDriver(new DatabaseSync(":memory:") as never); driver.exec(` @@ -108,9 +109,7 @@ function fetchReturning(bundles: FederatedSignalBundle[]): typeof fetch { describe("buildFederatedBenchmark() — not opted in", () => { it("returns null and touches neither the database nor the network for an absent/false manifest", async () => { - const fetchSpy = vi.fn(); - expect(await buildFederatedBenchmark(manifest({ enabled: false }), untouchableDb(), { fetchFn: fetchSpy })).toBeNull(); - expect(fetchSpy).not.toHaveBeenCalled(); + expect(await buildFederatedBenchmark(manifest({ enabled: false }), untouchableDb())).toBeNull(); }); it("returns null for a null manifest", async () => { @@ -118,12 +117,12 @@ describe("buildFederatedBenchmark() — not opted in", () => { }); }); -describe("buildFederatedBenchmark() — opted in, no peer data yet", () => { - it("returns local precision with peerCount 0 and a null median when no collector is configured", async () => { +describe("buildFederatedBenchmark() — opted in, no sync tick has run yet (cache miss)", () => { + it("returns local precision live, with peerCount 0 and a null median, when the cache is empty", async () => { const db = makeDb(); for (let pr = 1; pr <= 5; pr++) await resolved(db, pr); - const result = await buildFederatedBenchmark(manifest({ collectorUrl: null }), db, { now: NOW }); + const result = await buildFederatedBenchmark(manifest(), db, { now: NOW }); expect(result).not.toBeNull(); expect(result?.localMergePrecision).toBe(1); @@ -132,87 +131,165 @@ describe("buildFederatedBenchmark() — opted in, no peer data yet", () => { expect(result?.generatedAt).toBe("2026-07-16T00:00:00.000Z"); }); - it("returns peerCount 0 when every pulled bundle is rejected by trust-gating (untrusted key)", async () => { + it("honors an explicit windowDays override, narrowing the local calibration window", async () => { const db = makeDb(); - for (let pr = 1; pr <= 5; pr++) await resolved(db, pr); - const fetchFn = fetchReturning([signedWith(UNTRUSTED_KEY)]); + // Resolved 30 days before `now` — inside the default 90-day window but outside a 7-day override. + for (let pr = 1; pr <= 5; pr++) { + await resolved(db, pr, {}); + await db.prepare("UPDATE review_audit SET created_at = '2026-06-16T12:00:00Z' WHERE target_id = ?").bind(`owner/repo#${pr}`).run(); + } - const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW, fetchFn }); + const wide = await buildFederatedBenchmark(manifest(), db, { now: NOW, windowDays: 90 }); + const narrow = await buildFederatedBenchmark(manifest(), db, { now: NOW, windowDays: 7 }); - expect(result?.peerCount).toBe(0); - expect(result?.peerMedianMergePrecision).toBeNull(); + expect(wide?.localMergePrecision).toBe(1); + expect(narrow?.localMergePrecision).toBeNull(); }); - it("excludes an accepted peer bundle whose own mergePrecision is null (peer below its own MIN_DECIDED)", async () => { + it("uses Date.now() for generatedAt when opts.now is not provided", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(NOW)); + try { + const db = makeDb(); + const result = await buildFederatedBenchmark(manifest(), db, {}); + expect(result?.generatedAt).toBe("2026-07-16T00:00:00.000Z"); + } finally { + vi.useRealTimers(); + } + }); + + it("never touches the network — it only reads a system_flags cache row for the peer half", async () => { + // untouchableDb throws on ANY property access, including .prepare — proving the request path is DB-only + // (a single local read for the cache, plus buildFederatedBundle's own local query) and never calls fetch. + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + try { + const db = makeDb(); + await buildFederatedBenchmark(manifest(), db, { now: NOW }); + expect(fetchSpy).not.toHaveBeenCalled(); + } finally { + vi.unstubAllGlobals(); + } + }); +}); + +describe("buildFederatedBenchmark() — reads whatever refreshFederatedBenchmarkCache last wrote", () => { + it("reflects a fresh sync tick's median + peerCount on the very next read", async () => { const db = makeDb(); for (let pr = 1; pr <= 5; pr++) await resolved(db, pr); - const fetchFn = fetchReturning([signedWith(PEER_KEY_A, { mergePrecision: null })]); + const fetchFn = fetchReturning([signedWith(PEER_KEY_A, { mergePrecision: 0.6 })]); - const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW, fetchFn }); + await refreshFederatedBenchmarkCache(manifest(), db, { now: NOW, fetchFn }); + const result = await buildFederatedBenchmark(manifest(), db, { now: NOW }); - expect(result?.peerCount).toBe(0); - expect(result?.peerMedianMergePrecision).toBeNull(); + expect(result?.localMergePrecision).toBe(1); + expect(result?.peerMedianMergePrecision).toBe(0.6); + expect(result?.peerCount).toBe(1); }); -}); -describe("buildFederatedBenchmark() — opted in, local precision below MIN_DECIDED", () => { - it("reports a null local precision but still computes the peer median", async () => { + it("reports a null local precision (below MIN_DECIDED) while still surfacing a cached peer median", async () => { const db = makeDb(); await resolved(db, 1); // only 1 decided PR, below MIN_DECIDED (5) const fetchFn = fetchReturning([signedWith(PEER_KEY_A, { mergePrecision: 0.6 })]); - const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW, fetchFn }); + await refreshFederatedBenchmarkCache(manifest(), db, { now: NOW, fetchFn }); + const result = await buildFederatedBenchmark(manifest(), db, { now: NOW }); expect(result?.localMergePrecision).toBeNull(); expect(result?.peerMedianMergePrecision).toBe(0.6); expect(result?.peerCount).toBe(1); }); + + it("degrades to a null median / peerCount 0 when the cached row is corrupt JSON", async () => { + const db = makeDb(); + await db.prepare("INSERT INTO system_flags (key, value) VALUES ('orb:federated_benchmark_cache', 'not-json')").run(); + const result = await buildFederatedBenchmark(manifest(), db, { now: NOW }); + expect(result?.peerMedianMergePrecision).toBeNull(); + expect(result?.peerCount).toBe(0); + }); }); -describe("buildFederatedBenchmark() — opted in, real peer comparison", () => { +describe("refreshFederatedBenchmarkCache() — the background-tick write path (#9148)", () => { + it("is a no-op (cache untouched) when not opted in", async () => { + const db = makeDb(); + await db.prepare("INSERT INTO system_flags (key, value) VALUES ('orb:federated_benchmark_cache', ?)").bind(JSON.stringify({ peerMedianMergePrecision: 0.42, peerCount: 3, refreshedAt: "x" })).run(); + await refreshFederatedBenchmarkCache(manifest({ enabled: false }), db, { now: NOW }); + const row = await db.prepare("SELECT value FROM system_flags WHERE key = 'orb:federated_benchmark_cache'").first<{ value: string }>(); + expect(JSON.parse(row?.value ?? "{}")).toMatchObject({ peerMedianMergePrecision: 0.42, peerCount: 3 }); // untouched, not overwritten with an empty result + }); + + it("caches peerCount 0 and a null median when every pulled bundle is rejected by trust-gating", async () => { + const db = makeDb(); + const fetchFn = fetchReturning([signedWith(UNTRUSTED_KEY)]); + await refreshFederatedBenchmarkCache(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW, fetchFn }); + const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW }); + expect(result?.peerCount).toBe(0); + expect(result?.peerMedianMergePrecision).toBeNull(); + }); + + it("excludes an accepted peer bundle whose own mergePrecision is null (peer below its own MIN_DECIDED)", async () => { + const db = makeDb(); + const fetchFn = fetchReturning([signedWith(PEER_KEY_A, { mergePrecision: null })]); + await refreshFederatedBenchmarkCache(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW, fetchFn }); + const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW }); + expect(result?.peerCount).toBe(0); + expect(result?.peerMedianMergePrecision).toBeNull(); + }); + it("computes the median (not the mean) across every accepted peer, ignoring an untrusted one mixed in", async () => { const db = makeDb(); - for (let pr = 1; pr <= 5; pr++) await resolved(db, pr); const fetchFn = fetchReturning([ signedWith(PEER_KEY_A, { instanceId: "peer-a", mergePrecision: 0.5 }), signedWith(PEER_KEY_B, { instanceId: "peer-b", mergePrecision: 0.9 }), signedWith(UNTRUSTED_KEY, { instanceId: "peer-attacker", mergePrecision: 0.01 }), ]); - const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A, PEER_KEY_B] }), db, { now: NOW, fetchFn }); - + await refreshFederatedBenchmarkCache(manifest({ peerKeys: [PEER_KEY_A, PEER_KEY_B] }), db, { now: NOW, fetchFn }); // Median of [0.5, 0.9] (the untrusted 0.01 is rejected, not merely a low outlier) is 0.5 under this // module's nearest-rank percentile(50) (analytics.ts: idx = ceil(0.5*2)-1 = 0) — pinning the real // cross-module contract, not a re-derivation. + const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A, PEER_KEY_B] }), db, { now: NOW }); expect(result?.peerCount).toBe(2); expect(result?.peerMedianMergePrecision).toBe(0.5); - expect(result?.localMergePrecision).toBe(1); }); - it("honors an explicit windowDays override, narrowing the local calibration window", async () => { + it("a second tick's median REPLACES the first's, rather than merging the two", async () => { const db = makeDb(); - // Resolved 30 days before `now` — inside the default 90-day window but outside a 7-day override. - for (let pr = 1; pr <= 5; pr++) { - await resolved(db, pr, {}); - await db.prepare("UPDATE review_audit SET created_at = '2026-06-16T12:00:00Z' WHERE target_id = ?").bind(`owner/repo#${pr}`).run(); - } + await refreshFederatedBenchmarkCache(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW, fetchFn: fetchReturning([signedWith(PEER_KEY_A, { instanceId: "peer-a", mergePrecision: 0.2 })]) }); + await refreshFederatedBenchmarkCache(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW + 60_000, fetchFn: fetchReturning([signedWith(PEER_KEY_A, { instanceId: "peer-a", generatedAt: new Date(NOW + 60_000).toISOString(), mergePrecision: 0.9 })]) }); + const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A] }), db, { now: NOW }); + expect(result?.peerMedianMergePrecision).toBe(0.9); + }); +}); - const wide = await buildFederatedBenchmark(manifest({ collectorUrl: null }), db, { now: NOW, windowDays: 90 }); - const narrow = await buildFederatedBenchmark(manifest({ collectorUrl: null }), db, { now: NOW, windowDays: 7 }); +describe("runFederatedPeerSyncTick() — the full tick: cache refresh + own-bundle push (#9148/#9166)", () => { + it("refreshes the cache AND posts this instance's own bundle to the configured collector", async () => { + const db = makeDb(); + for (let pr = 1; pr <= 5; pr++) await resolved(db, pr); + const posted: { url: string }[] = []; + const fetchFn = (async (url: string, init?: RequestInit) => { + posted.push({ url: String(url) }); + if (init?.method === "POST") return new Response("ok"); + return Response.json([signedWith(PEER_KEY_A, { mergePrecision: 0.6 })]); + }) as unknown as typeof fetch; - expect(wide?.localMergePrecision).toBe(1); - expect(narrow?.localMergePrecision).toBeNull(); + await runFederatedPeerSyncTick(manifest(), db, { now: NOW, fetchFn }); + + expect(posted.some((p) => p.url === URL_OK)).toBe(true); // both push and pull hit the same configured collector + const result = await buildFederatedBenchmark(manifest(), db, { now: NOW }); + expect(result?.peerMedianMergePrecision).toBe(0.6); }); - it("uses Date.now() for generatedAt when opts.now is not provided", async () => { - vi.useFakeTimers(); - vi.setSystemTime(new Date(NOW)); - try { - const db = makeDb(); - const result = await buildFederatedBenchmark(manifest({ collectorUrl: null }), db, {}); - expect(result?.generatedAt).toBe("2026-07-16T00:00:00.000Z"); - } finally { - vi.useRealTimers(); - } + it("a push failure never skips the pull-side cache refresh (allSettled, not all)", async () => { + const db = makeDb(); + const fetchFn = (async (_url: string, init?: RequestInit) => { + if (init?.method === "POST") throw new Error("collector down"); + return Response.json([signedWith(PEER_KEY_A, { mergePrecision: 0.6 })]); + }) as unknown as typeof fetch; + + await runFederatedPeerSyncTick(manifest(), db, { now: NOW, fetchFn, maxAttempts: 1, sleepFn: () => Promise.resolve() }); + + const result = await buildFederatedBenchmark(manifest(), db, { now: NOW }); + expect(result?.peerMedianMergePrecision).toBe(0.6); // pull side still completed and cached }); }); diff --git a/test/unit/federated-bundle.test.ts b/test/unit/federated-bundle.test.ts index 7dae9e4516..b1f2543b04 100644 --- a/test/unit/federated-bundle.test.ts +++ b/test/unit/federated-bundle.test.ts @@ -2,11 +2,12 @@ import { DatabaseSync } from "node:sqlite"; import { createHmac } from "node:crypto"; import { describe, expect, it, vi } from "vitest"; import { createD1Adapter, nodeSqliteDriver } from "../../src/selfhost/d1-adapter"; -import { getOrCreateAnonSecret, instanceId } from "../../src/selfhost/orb-collector"; +import { getOrCreateAnonSecret, getOrCreateInstanceIdentitySecret, instanceId } from "../../src/selfhost/orb-collector"; import { FEDERATED_BUNDLE_SCHEMA_VERSION, buildFederatedBundle, canonicalizeFederatedBundleBody, + getOrCreateFederatedSigningSecret, isFederatedIntelligenceEnabled, signFederatedBundle, type FederatedSignalBundle, @@ -153,13 +154,15 @@ describe("buildFederatedBundle() — opted in", () => { expect(b.cycleP50Ms).toBe(7_200_000); // 2h decision → outcome expect(b.cycleP95Ms).toBe(7_200_000); - // The opaque handle is the orb pipeline's, not a second identity. - expect(b.instanceId).toBe(instanceId(await getOrCreateAnonSecret(db))); + // The opaque handle is the orb pipeline's, not a second identity — but (#9147) derived from the DEDICATED + // identity secret, not the anon secret. + expect(b.instanceId).toBe(instanceId(await getOrCreateInstanceIdentitySecret(db))); - // Independently recompute the HMAC over the canonical body — the signature must verify. + // Independently recompute the HMAC over the canonical body — the signature must verify against the + // DEDICATED federated signing secret (#9147), never the anon secret. const { signature, ...body } = b; - const secret = await getOrCreateAnonSecret(db); - expect(signature).toBe(createHmac("sha256", secret).update(canonicalizeFederatedBundleBody(body)).digest("hex")); + const signingSecret = await getOrCreateFederatedSigningSecret(db); + expect(signature).toBe(createHmac("sha256", signingSecret).update(canonicalizeFederatedBundleBody(body)).digest("hex")); expect(signature).toMatch(/^[0-9a-f]{64}$/); // The export path itself never talks to anyone — transport is #6479. @@ -312,6 +315,40 @@ describe("buildFederatedBundle() — fail-safe", () => { }); }); +describe("#9147: federated signing key / anon secret separation", () => { + it("mints a federated signing secret that is never the same value as the anon secret", async () => { + const db = makeDb(); + const anonSecret = await getOrCreateAnonSecret(db); + const signingSecret = await getOrCreateFederatedSigningSecret(db); + expect(signingSecret).not.toBe(anonSecret); + expect(signingSecret).toMatch(/^[0-9a-f]{64}$/); + // Stable across repeated reads, exactly like getOrCreateAnonSecret. + expect(await getOrCreateFederatedSigningSecret(db)).toBe(signingSecret); + }); + + it("keeps instanceId stable when the anon secret is rotated after the identity secret already exists", async () => { + const db = makeDb(); + await resolved(db, 1); + const before = (await buildFederatedBundle(manifest(true), db, { now: NOW })) as FederatedSignalBundle; + + // Simulate an operator rotating the anonymization secret (e.g. after a suspected leak) — this must never + // change the instance's federated identity, since that would break the receiver's per-instance dedup. + await db.prepare("UPDATE system_flags SET value = ? WHERE key = 'orb:anon_secret'").bind("f".repeat(64)).run(); + expect(await getOrCreateAnonSecret(db)).toBe("f".repeat(64)); + + const after = (await buildFederatedBundle(manifest(true), db, { now: NOW })) as FederatedSignalBundle; + expect(after.instanceId).toBe(before.instanceId); + }); + + it("seeds the identity secret from the CURRENT anon secret on first use (no disruptive instanceId change on upgrade)", async () => { + const db = makeDb(); + const anonSecret = await getOrCreateAnonSecret(db); + // No identity-secret row exists yet — getOrCreateInstanceIdentitySecret must seed from the anon secret so + // an instance upgrading from before #9147 keeps its existing instanceId. + expect(await getOrCreateInstanceIdentitySecret(db)).toBe(anonSecret); + }); +}); + describe("canonicalizeFederatedBundleBody() / signFederatedBundle()", () => { it("is insensitive to key insertion order, so a receiver can recompute the HMAC byte-for-byte", async () => { const db = makeDb(); diff --git a/test/unit/federated-collector.test.ts b/test/unit/federated-collector.test.ts index 80b443b8fb..5f801985e3 100644 --- a/test/unit/federated-collector.test.ts +++ b/test/unit/federated-collector.test.ts @@ -1,8 +1,9 @@ import { DatabaseSync } from "node:sqlite"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createD1Adapter, nodeSqliteDriver } from "../../src/selfhost/d1-adapter"; import { FEDERATED_BUNDLE_SCHEMA_VERSION, type FederatedSignalBundle } from "../../src/orb/federated-bundle"; import { + __resetFederatedCollectorRateLimitForTest, pullPeerBundles, pushFederatedBundle, resolveCollectorEndpoint, @@ -10,6 +11,13 @@ import { } from "../../src/orb/federated-collector"; import type { FederatedCollectorMode, FocusManifest } from "../../src/signals/focus-manifest"; +// #9166: pullPeerBundles/pushFederatedBundle now fall back to a module-level default rate-limit bucket when +// no `opts.bucket` is supplied. Reset it between tests so the many call sites in this file that omit a +// bucket (deliberately, to exercise the "no bucket" default arm) don't leak count across test cases. +afterEach(() => { + __resetFederatedCollectorRateLimitForTest(); +}); + const URL_OK = "https://collector.example.org/v1/federated"; /** In-memory DB with the tables the bundle builder reads (mirrors federated-bundle.test.ts's makeDb). */ @@ -104,8 +112,12 @@ function bundle(over: Partial = {}): Record= 200 && status < 300, status, json: async () => body } as unknown as Response; + return new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } }); } describe("resolveCollectorEndpoint()", () => { @@ -229,18 +241,8 @@ describe("push/pull — failure handling never reaches the gate", () => { }); it("returns [] when the response body is not JSON at all", async () => { - const err = vi.spyOn(console, "error").mockImplementation(() => {}); - const fetchFn = (async () => - ({ - ok: true, - status: 200, - json: async () => { - throw new SyntaxError("Unexpected token < in JSON"); - }, - }) as unknown as Response) as unknown as typeof fetch; + const fetchFn = (async () => new Response("not json", { status: 200 })) as unknown as typeof fetch; await expect(pullPeerBundles(manifest(), { ...DETERMINISTIC, fetchFn })).resolves.toEqual([]); - expect(err).toHaveBeenCalled(); - err.mockRestore(); }); }); @@ -278,6 +280,21 @@ describe("pullPeerBundles()", () => { expect(got[0]!.signature).toBe("f".repeat(64)); }); + it("#9148: caps the number of pulled entries even shape-checked, independent of the byte ceiling", async () => { + const many = Array.from({ length: 250 }, (_, i) => bundle({ instanceId: `inst-${i}` })); + const fetchFn = (async () => jsonResponse(many)) as unknown as typeof fetch; + const got = await pullPeerBundles(manifest(), { ...DETERMINISTIC, fetchFn }); + expect(got).toHaveLength(200); // MAX_PULLED_BUNDLES, not the 250 the collector claimed to have + expect(got[0]!.instanceId).toBe("inst-0"); + expect(got[199]!.instanceId).toBe("inst-199"); + }); + + it("#9148: rejects a response over the ingest body's byte ceiling instead of buffering it", async () => { + const oversized = JSON.stringify([bundle()]).padEnd(2 * 1_048_576, " "); + const fetchFn = (async () => new Response(oversized, { status: 200 })) as unknown as typeof fetch; + expect(await pullPeerBundles(manifest(), { ...DETERMINISTIC, fetchFn })).toEqual([]); + }); + it("returns [] for a non-array payload", async () => { const fetchFn = (async () => jsonResponse({ bundles: [bundle()] })) as unknown as typeof fetch; expect(await pullPeerBundles(manifest(), { ...DETERMINISTIC, fetchFn })).toEqual([]); @@ -302,6 +319,70 @@ describe("rate limiting — a best-effort sync must not hammer a peer", () => { expect(await pushFederatedBundle(manifest(), db, { ...DETERMINISTIC, fetchFn, bucket: fresh })).toBe(true); expect(await pullPeerBundles(manifest(), { ...DETERMINISTIC, fetchFn })).toEqual([]); }); + + // #9166: the rate limiter was inert at every real call site because the only production caller + // (buildFederatedBenchmark) never supplied a bucket, and `!opts.bucket` meant "unlimited". These prove the + // module-level default actually engages and blocks once exhausted, with no caller-supplied bucket at all. + describe("#9166: the default bucket (no opts.bucket supplied) actually engages", () => { + it("blocks pull after the limit within one window, using only the module-level default", async () => { + const fetchFn = (async () => jsonResponse([])) as unknown as typeof fetch; + let calls = 0; + const countingFetch = (async (...args: Parameters) => { + calls += 1; + return fetchFn(...args); + }) as unknown as typeof fetch; + for (let i = 0; i < 6; i++) { + expect(await pullPeerBundles(manifest(), { now: NOW, fetchFn: countingFetch })).toEqual([]); + } + expect(calls).toBe(6); // all 6 within the limit went through + // The 7th, same window, same instant — no bucket supplied by the caller — must now be blocked. + expect(await pullPeerBundles(manifest(), { now: NOW, fetchFn: countingFetch })).toEqual([]); + expect(calls).toBe(6); // the 7th never reached fetch at all + }); + + it("blocks push after the limit within one window, using only the module-level default", async () => { + const db = makeDb(); + await resolved(db, 1); + let calls = 0; + const countingFetch = (async () => { + calls += 1; + return jsonResponse({ ok: true }); + }) as unknown as typeof fetch; + for (let i = 0; i < 6; i++) { + expect(await pushFederatedBundle(manifest(), db, { now: NOW, fetchFn: countingFetch })).toBe(true); + } + expect(calls).toBe(6); + expect(await pushFederatedBundle(manifest(), db, { now: NOW, fetchFn: countingFetch })).toBe(false); + expect(calls).toBe(6); + }); + + it("push and pull are independent default buckets — exhausting one never blocks the other", async () => { + const db = makeDb(); + await resolved(db, 1); + const fetchFn = (async () => jsonResponse({ ok: true })) as unknown as typeof fetch; + for (let i = 0; i < 6; i++) expect(await pushFederatedBundle(manifest(), db, { now: NOW, fetchFn })).toBe(true); + expect(await pushFederatedBundle(manifest(), db, { now: NOW, fetchFn })).toBe(false); // push exhausted + + const pullFetch = (async () => jsonResponse([])) as unknown as typeof fetch; + expect(await pullPeerBundles(manifest(), { now: NOW, fetchFn: pullFetch })).toEqual([]); // pull untouched + }); + + it("resets after the window elapses", async () => { + const fetchFn = (async () => jsonResponse([])) as unknown as typeof fetch; + for (let i = 0; i < 6; i++) expect(await pullPeerBundles(manifest(), { now: NOW, fetchFn })).toEqual([]); + // Same instant again — still exhausted. + let calls = 0; + const countingFetch = (async () => { + calls += 1; + return jsonResponse([]); + }) as unknown as typeof fetch; + expect(await pullPeerBundles(manifest(), { now: NOW, fetchFn: countingFetch })).toEqual([]); + expect(calls).toBe(0); + // A full window later, the bucket resets and the request goes through again. + expect(await pullPeerBundles(manifest(), { now: NOW + 61_000, fetchFn: countingFetch })).toEqual([]); + expect(calls).toBe(1); + }); + }); }); describe("defaults", () => { diff --git a/test/unit/federated-import.test.ts b/test/unit/federated-import.test.ts index 17c583c4c3..e911c569e7 100644 --- a/test/unit/federated-import.test.ts +++ b/test/unit/federated-import.test.ts @@ -3,11 +3,14 @@ import { describe, expect, it, vi } from "vitest"; import { canonicalizeFederatedBundleBody, FEDERATED_BUNDLE_SCHEMA_VERSION, type FederatedSignalBundle } from "../../src/orb/federated-bundle"; import { + applyFederatedPeerWatermarks, importPeerBundles, isFederatedImportEnabled, + matchingFederatedKey, verifyFederatedBundle, type FederatedRejection, } from "../../src/orb/federated-import"; +import { createTestEnv, type TestD1Database } from "../helpers/d1"; import type { FocusManifest } from "../../src/signals/focus-manifest"; // Fake 64-hex keys — the shape generateAnonSecret produces. Not secrets: locally-invented test fixtures. @@ -15,10 +18,16 @@ const PEER_KEY_A = "a".repeat(64); const PEER_KEY_B = "b".repeat(64); const UNTRUSTED_KEY = "c".repeat(64); +// Computed at call time (not a fixed literal) so every test bundle is fresh relative to whatever `now` +// importPeerBundles defaults to (real Date.now()) — the freshness gate (#9148) would otherwise reject a +// fixed past date as the test suite ages. Tests that specifically exercise staleness pass an explicit +// `generatedAt` (and/or an explicit `now` opt) instead of relying on this default. +const FRESH_GENERATED_AT = () => new Date(Date.now() - 60_000).toISOString(); + const body = (over: Partial = {}) => ({ schemaVersion: FEDERATED_BUNDLE_SCHEMA_VERSION, instanceId: "abc123def4567890", - generatedAt: "2026-02-01T00:00:00.000Z", + generatedAt: FRESH_GENERATED_AT(), windowDays: 90, decided: 40, mergePrecision: 0.9, @@ -218,4 +227,182 @@ describe("importPeerBundles (#6480)", () => { expect(importPeerBundles(null, [signedWith(PEER_KEY_A)], { log: () => undefined }).rejected[0]!.reason).toBe("not_opted_in"); expect(importPeerBundles(undefined, [], { log: () => undefined })).toEqual({ accepted: [], rejected: [] }); }); + + it("#9148: keeps only the LAST bundle per instanceId within a batch (Sybil dedup)", () => { + const first = signedWith(PEER_KEY_A, { mergePrecision: 0.5 }); + const second = signedWith(PEER_KEY_A, { mergePrecision: 0.99 }); // same instanceId, re-signed + const result = importPeerBundles(manifest(), [first, second], { log: () => undefined }); + expect(result.accepted).toEqual([second]); + expect(result.rejected).toEqual([]); // the superseded duplicate is silently dropped, not logged as a rejection + }); + + it("#9148: rejects every numeric field outside its valid range", () => { + const cases: Partial[] = [ + { mergePrecision: -0.01 }, + { mergePrecision: 1.01 }, + { closePrecision: -1 }, + { fpRate: 2 }, + { fnRate: -0.5 }, + { reversalRate: 1.5 }, + { slopRate: -0.1 }, + { copycatRate: 1.5 }, + { decided: -1 }, + { windowDays: 0 }, + { windowDays: -90 }, + { cycleP50Ms: -1 }, + { cycleP95Ms: -1 }, + { cycleP50Ms: 5000, cycleP95Ms: 1000 }, // p50 can never exceed p95 of the same sample + ]; + for (const over of cases) { + const result = importPeerBundles(manifest(), [signedWith(PEER_KEY_A, over)], { log: () => undefined }); + expect(result.rejected[0]?.reason, JSON.stringify(over)).toBe("out_of_range"); + } + }); + + it("#9148: accepts the boundary values 0 and 1 for a unit-rate field (inclusive range)", () => { + const zero = importPeerBundles(manifest(), [signedWith(PEER_KEY_A, { instanceId: "i-zero", mergePrecision: 0, fpRate: 0 })], { log: () => undefined }); + expect(zero.accepted).toHaveLength(1); + const one = importPeerBundles(manifest(), [signedWith(PEER_KEY_A, { instanceId: "i-one", mergePrecision: 1, fpRate: 1 })], { log: () => undefined }); + expect(one.accepted).toHaveLength(1); + }); + + it("#9148: rejects a windowDays that does not match the local instance's own resolved window", () => { + const result = importPeerBundles(manifest(), [signedWith(PEER_KEY_A, { windowDays: 30 })], { log: () => undefined }, ); + expect(result.rejected).toEqual([{ instanceId: "abc123def4567890", reason: "window_mismatch" }]); + }); + + it("#9148: honors an explicit localWindowDays override instead of the DEFAULT_WINDOW_DAYS default", () => { + const bundle = signedWith(PEER_KEY_A, { windowDays: 30 }); + const result = importPeerBundles(manifest(), [bundle], { log: () => undefined, localWindowDays: 30 }); + expect(result.accepted).toEqual([bundle]); + }); + + it("#9148: rejects an unparseable generatedAt", () => { + const result = importPeerBundles(manifest(), [signedWith(PEER_KEY_A, { generatedAt: "not-a-date" })], { log: () => undefined }); + expect(result.rejected).toEqual([{ instanceId: "abc123def4567890", reason: "stale_or_future" }]); + }); + + it("#9148: rejects a bundle older than the max age, and one too far in the future — accepts one just inside each edge", () => { + const now = Date.parse("2026-03-01T00:00:00.000Z"); + const eightDaysStale = signedWith(PEER_KEY_A, { generatedAt: "2026-02-21T00:00:00.000Z" }); // > 7 days old + const sixDaysStale = signedWith(PEER_KEY_A, { generatedAt: "2026-02-23T00:00:00.000Z" }); // < 7 days old + const tenMinutesFuture = signedWith(PEER_KEY_A, { generatedAt: "2026-03-01T00:10:00.000Z" }); // > 5 min skew + const oneMinuteFuture = signedWith(PEER_KEY_A, { generatedAt: "2026-03-01T00:01:00.000Z" }); // < 5 min skew + expect(importPeerBundles(manifest(), [eightDaysStale], { log: () => undefined, now }).rejected[0]?.reason).toBe("stale_or_future"); + expect(importPeerBundles(manifest(), [sixDaysStale], { log: () => undefined, now }).accepted).toEqual([sixDaysStale]); + expect(importPeerBundles(manifest(), [tenMinutesFuture], { log: () => undefined, now }).rejected[0]?.reason).toBe("stale_or_future"); + expect(importPeerBundles(manifest(), [oneMinuteFuture], { log: () => undefined, now }).accepted).toEqual([oneMinuteFuture]); + }); + + it("#9148: enforces MIN_DECIDED receiver-side, regardless of what the sender claims", () => { + // A hostile/buggy peer self-reporting a real mergePrecision despite decided being below MIN_DECIDED (5). + const underclaimed = signedWith(PEER_KEY_A, { decided: 3, mergePrecision: 1.0 }); + const result = importPeerBundles(manifest(), [underclaimed], { log: () => undefined }); + expect(result.rejected).toEqual([{ instanceId: "abc123def4567890", reason: "below_min_decided" }]); + }); + + it("#9148: accepts a decided count exactly at MIN_DECIDED", () => { + const bundle = signedWith(PEER_KEY_A, { decided: 5 }); + expect(importPeerBundles(manifest(), [bundle], { log: () => undefined }).accepted).toEqual([bundle]); + }); +}); + +describe("matchingFederatedKey (#9148)", () => { + it("returns the specific key that verified, not just a boolean", () => { + expect(matchingFederatedKey(signedWith(PEER_KEY_B), [PEER_KEY_A, PEER_KEY_B])).toBe(PEER_KEY_B); + }); + + it("returns null when no allowlisted key verifies", () => { + expect(matchingFederatedKey(signedWith(UNTRUSTED_KEY), [PEER_KEY_A, PEER_KEY_B])).toBeNull(); + }); +}); + +describe("applyFederatedPeerWatermarks (#9148, the persisted gates)", () => { + const env = () => createTestEnv(); + const db = (e: Env) => e.DB as unknown as TestD1Database; + const okResult = (bundles: FederatedSignalBundle[]): { accepted: FederatedSignalBundle[]; rejected: [] } => ({ accepted: bundles, rejected: [] }); + + it("admits a brand-new instanceId and persists its watermark", async () => { + const e = env(); + const bundle = signedWith(PEER_KEY_A); + const result = await applyFederatedPeerWatermarks(e.DB, okResult([bundle]), [PEER_KEY_A], { now: Date.parse(bundle.generatedAt) }); + expect(result.accepted).toEqual([bundle]); + expect(result.rejected).toEqual([]); + const row = await db(e).prepare("SELECT value FROM system_flags WHERE key = 'orb:federated_peer_state'").first<{ value: string }>(); + expect(JSON.parse(row?.value ?? "{}")).toHaveProperty(bundle.instanceId); + }); + + it("admits a newer bundle for an already-known instanceId, refreshing the watermark", async () => { + const e = env(); + const first = signedWith(PEER_KEY_A, { generatedAt: "2026-03-01T00:00:00.000Z" }); + await applyFederatedPeerWatermarks(e.DB, okResult([first]), [PEER_KEY_A], { now: Date.parse(first.generatedAt) }); + const second = signedWith(PEER_KEY_A, { generatedAt: "2026-03-02T00:00:00.000Z" }); + const result = await applyFederatedPeerWatermarks(e.DB, okResult([second]), [PEER_KEY_A], { now: Date.parse(second.generatedAt) }); + expect(result.accepted).toEqual([second]); + }); + + it("rejects a replay: the same or an older generatedAt for an instanceId already on record", async () => { + const e = env(); + const fresh = signedWith(PEER_KEY_A, { generatedAt: "2026-03-05T00:00:00.000Z" }); + await applyFederatedPeerWatermarks(e.DB, okResult([fresh]), [PEER_KEY_A], { now: Date.parse(fresh.generatedAt) }); + const replay = signedWith(PEER_KEY_A, { generatedAt: "2026-03-01T00:00:00.000Z" }); // same instanceId, OLDER generatedAt + const result = await applyFederatedPeerWatermarks(e.DB, okResult([replay]), [PEER_KEY_A], { now: Date.parse(fresh.generatedAt) }); + expect(result.accepted).toEqual([]); + expect(result.rejected).toEqual([{ instanceId: "abc123def4567890", reason: "replayed_or_rollback" }]); + }); + + it("caps a single key at MAX_INSTANCES_PER_KEY distinct instanceIds, admitting the first N and rejecting the rest", async () => { + const e = env(); + let accepted: FederatedSignalBundle[] = []; + let lastRejectedReason: string | undefined; + for (let i = 0; i < 11; i += 1) { + const bundle = signedWith(PEER_KEY_A, { instanceId: `sybil-instance-${i}` }); + const result = await applyFederatedPeerWatermarks(e.DB, okResult([bundle]), [PEER_KEY_A], { now: Date.parse(bundle.generatedAt) }); + accepted = accepted.concat(result.accepted); + if (result.rejected.length > 0) lastRejectedReason = result.rejected[0]?.reason; + } + expect(accepted).toHaveLength(10); // MAX_INSTANCES_PER_KEY + expect(lastRejectedReason).toBe("sybil_cap_exceeded"); + }); + + it("a DIFFERENT verifying key gets its own independent cap", async () => { + const e = env(); + for (let i = 0; i < 10; i += 1) { + const bundle = signedWith(PEER_KEY_A, { instanceId: `key-a-instance-${i}` }); + await applyFederatedPeerWatermarks(e.DB, okResult([bundle]), [PEER_KEY_A, PEER_KEY_B], { now: Date.parse(bundle.generatedAt) }); + } + // PEER_KEY_A is now at its cap, but PEER_KEY_B has contributed nothing yet — a new instance under B must + // still be admitted, proving the cap is scoped per-key rather than a single shared global counter. + const underB = signedWith(PEER_KEY_B, { instanceId: "key-b-instance-0" }); + const result = await applyFederatedPeerWatermarks(e.DB, okResult([underB]), [PEER_KEY_A, PEER_KEY_B], { now: Date.parse(underB.generatedAt) }); + expect(result.accepted).toEqual([underB]); + }); + + it("prunes a peer-state entry that hasn't been refreshed in a very long time, freeing its key's cap slot", async () => { + const e = env(); + const veryOldEntry = { "stale-instance": { keyFingerprint: "x".repeat(64), lastGeneratedAtMs: 0, lastSeenAtMs: 0, firstSeenAtMs: 0 } }; + await db(e).prepare("INSERT INTO system_flags (key, value, updated_at) VALUES ('orb:federated_peer_state', ?, CURRENT_TIMESTAMP)").bind(JSON.stringify(veryOldEntry)).run(); + const bundle = signedWith(PEER_KEY_A, { instanceId: "fresh-instance" }); + const farFuture = Date.parse(bundle.generatedAt) + 500 * 86_400_000; // well past PEER_STATE_PRUNE_AFTER_MS + await applyFederatedPeerWatermarks(e.DB, okResult([bundle]), [PEER_KEY_A], { now: farFuture }); + const row = await db(e).prepare("SELECT value FROM system_flags WHERE key = 'orb:federated_peer_state'").first<{ value: string }>(); + expect(JSON.parse(row?.value ?? "{}")).not.toHaveProperty("stale-instance"); // pruned, not just superseded + }); + + it("degrades to treating every instanceId as new when the persisted state is corrupt JSON", async () => { + const e = env(); + await db(e).prepare("INSERT INTO system_flags (key, value, updated_at) VALUES ('orb:federated_peer_state', 'not-json', CURRENT_TIMESTAMP)").run(); + const bundle = signedWith(PEER_KEY_A); + const result = await applyFederatedPeerWatermarks(e.DB, okResult([bundle]), [PEER_KEY_A], { now: Date.parse(bundle.generatedAt) }); + expect(result.accepted).toEqual([bundle]); // fail-safe: corrupt cache never blocks a legitimate peer + }); + + it("passes through prior rejections untouched alongside its own", async () => { + const e = env(); + const priorRejection: FederatedRejection = { instanceId: "already-rejected", reason: "malformed" }; + const bundle = signedWith(PEER_KEY_A); + const result = await applyFederatedPeerWatermarks(e.DB, { accepted: [bundle], rejected: [priorRejection] }, [PEER_KEY_A], { now: Date.parse(bundle.generatedAt) }); + expect(result.rejected).toContainEqual(priorRejection); + expect(result.accepted).toEqual([bundle]); + }); }); diff --git a/test/unit/index.test.ts b/test/unit/index.test.ts index eb0f076786..2056d24ecc 100644 --- a/test/unit/index.test.ts +++ b/test/unit/index.test.ts @@ -4,11 +4,13 @@ import { recordGitHubRateLimitObservation } from "../../src/db/repositories"; import { scheduledEnqueueDelaySeconds } from "../../src/selfhost/queue-common"; import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; import { clearOpsManifestOverrideCacheForTest } from "../../src/review/ops-wire"; +import { clearFederatedIntelligenceManifestOverrideCacheForTest } from "../../src/orb/federated-benchmark"; import { createTestEnv } from "../helpers/d1"; describe("worker entrypoint", () => { beforeEach(() => { clearOpsManifestOverrideCacheForTest(); + clearFederatedIntelligenceManifestOverrideCacheForTest(); }); afterEach(() => { vi.restoreAllMocks(); @@ -1002,6 +1004,37 @@ describe("worker entrypoint", () => { expect(sent.some((m) => m.type === "reconcile-active-review-tracking")).toBe(false); }); + it("enqueues federated-peer-sync every 10 minutes ONLY when the loopover self-repo manifest opts in (#9148) — absent-manifest default is byte-identical", async () => { + const sentFor = async (enabled?: boolean, isoTime = "2026-05-25T05:10:00.000Z"): Promise> => { + clearFederatedIntelligenceManifestOverrideCacheForTest(); // each sub-case below is a fresh env; don't let the prior case's cached override leak in + const sent: Array = []; + const env = createTestEnv({ LOOPOVER_DRIFT_ISSUE_REPO: "JSONbored/loopover", JOBS: { async send(message: import("../../src/types").JobMessage) { sent.push(message); } } as unknown as Queue }); + if (enabled !== undefined) await upsertRepoFocusManifest(env, "JSONbored/loopover", { federatedIntelligence: { enabled } }); + const waitUntil: Promise[] = []; + await worker.scheduled(controllerFor(isoTime), env, executionContext(waitUntil)); + await Promise.all(waitUntil); + return sent; + }; + + // No manifest block at all (absent federatedIntelligence, the fleet-wide default) → never enqueued. + expect((await sentFor(undefined)).some((m) => m.type === "federated-peer-sync")).toBe(false); + // Manifest present but explicitly disabled → still never enqueued. + expect((await sentFor(false)).some((m) => m.type === "federated-peer-sync")).toBe(false); + // Opted in, on a 10-minute boundary → exactly one federated-peer-sync job. + const on = await sentFor(true); + expect(on.filter((m) => m.type === "federated-peer-sync")).toEqual([{ type: "federated-peer-sync", requestedBy: "schedule" }]); + }); + + it("does NOT enqueue federated-peer-sync outside the 10-minute window even when opted in", async () => { + const sent: Array = []; + const env = createTestEnv({ LOOPOVER_DRIFT_ISSUE_REPO: "JSONbored/loopover", JOBS: { async send(message: import("../../src/types").JobMessage) { sent.push(message); } } as unknown as Queue }); + await upsertRepoFocusManifest(env, "JSONbored/loopover", { federatedIntelligence: { enabled: true } }); + const waitUntil: Promise[] = []; + await worker.scheduled(controllerFor("2026-05-25T05:14:00.000Z"), env, executionContext(waitUntil)); // not a 10-minute boundary + await Promise.all(waitUntil); + expect(sent.some((m) => m.type === "federated-peer-sync")).toBe(false); + }); + it("enqueues selftune hourly only when LOOPOVER_REVIEW_SELFTUNE is ON", async () => { const sentFor = async ( selfTuneFlag?: string, diff --git a/test/unit/job-dispatch.test.ts b/test/unit/job-dispatch.test.ts index 8a6e95e59f..f2a51b4032 100644 --- a/test/unit/job-dispatch.test.ts +++ b/test/unit/job-dispatch.test.ts @@ -3,6 +3,8 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { processJob } from "../../src/queue/job-dispatch"; import { createTestEnv } from "../helpers/d1"; import { upsertInstallation, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; +import { clearFederatedIntelligenceManifestOverrideCacheForTest } from "../../src/orb/federated-benchmark"; import type { JobMessage } from "../../src/types"; describe("processJob unknown job type (#5836)", () => { @@ -41,6 +43,34 @@ describe("processJob unknown job type (#5836)", () => { }); }); +describe("processJob federated-peer-sync (#9148/#9166)", () => { + afterEach(() => { + clearFederatedIntelligenceManifestOverrideCacheForTest(); + vi.unstubAllGlobals(); + }); + + it("is a no-op — never writes the peer-benchmark cache — when the loopover self-repo manifest has not opted in", async () => { + const env = createTestEnv({ LOOPOVER_DRIFT_ISSUE_REPO: "JSONbored/loopover" }); + // The manifest override lookup itself may still fall through to a network read when nothing is cached + // (the same "config-as-code override" pattern every sibling scheduled job uses, e.g. resolveOpsManifestOverride) + // — stub it to a harmless empty response so this test exercises the GATE, not GitHub reachability. + vi.stubGlobal("fetch", vi.fn(async () => new Response("null", { status: 200 }))); + await expect(processJob(env, { type: "federated-peer-sync", requestedBy: "schedule" } as JobMessage)).resolves.toBeUndefined(); + const row = await env.DB.prepare("SELECT 1 AS x FROM system_flags WHERE key = 'orb:federated_benchmark_cache'").first(); + expect(row ?? null).toBeNull(); + }); + + it("re-checks the FULL manifest at dispatch time and runs the sync tick when opted in, defense-in-depth against a stale enqueue", async () => { + const env = createTestEnv({ LOOPOVER_DRIFT_ISSUE_REPO: "JSONbored/loopover" }); + // No collectorUrl configured, so both push and pull resolve to a no-op with zero fetch calls — this proves + // the job runs the real tick (which writes the peer cache) without needing a live network mock. + await upsertRepoFocusManifest(env, "JSONbored/loopover", { federatedIntelligence: { enabled: true } }); + await expect(processJob(env, { type: "federated-peer-sync", requestedBy: "schedule" } as JobMessage)).resolves.toBeUndefined(); + const row = await env.DB.prepare("SELECT value FROM system_flags WHERE key = 'orb:federated_benchmark_cache'").first<{ value: string }>(); + expect(JSON.parse(row?.value ?? "{}")).toMatchObject({ peerCount: 0, peerMedianMergePrecision: null }); + }); +}); + describe("processJob backfill-registered-repos fan-out isolation (#8355)", () => { afterEach(() => { vi.restoreAllMocks();