Skip to content

fix(orb): bind identity to credentials instead of renameable logins or trusted request bodies - #9177

Merged
JSONbored merged 1 commit into
mainfrom
fix/identity-binding-9121-9125-9126-9131
Jul 27, 2026
Merged

fix(orb): bind identity to credentials instead of renameable logins or trusted request bodies#9177
JSONbored merged 1 commit into
mainfrom
fix/identity-binding-9121-9125-9126-9131

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Four contributor-scoped controls trusted a mutable, attacker-influenced identity signal instead of a stable, authenticated one. This PR binds each to something the subject cannot unilaterally change or forge.

Closes #9121
Closes #9125
Closes #9126
Closes #9131

Details

Scope

Two new migrations (0187_orb_instance_credentials.sql, 0188_pull_request_issue_author_github_id.sql, 0189_submitter_outcome_log.sql), scripts/check-schema-drift.ts allowlist updated for the two new raw-SQL tables, apps/loopover-ui/src/lib/selfhost-env-reference.ts regenerated for the new ORB_COLLECTOR_INSTANCE_SECRET env var.

Validation

  • npm run typecheck — clean
  • npm run db:migrations:check / npm run db:schema-drift:check — clean
  • npm run selfhost:env-reference:check — clean
  • npm run manifest:drift-check / npm run ui:openapi:check / npm run coverage-boltons:check — clean
  • Targeted vitest run --coverage across every changed src/** file (auth/security.ts, orb/ingest.ts, review/{submitter-reputation,reputation-wire,public-stats}.ts, services/control-panel-roles.ts, plus the touched slices of db/repositories.ts and queue/processors.ts) — 100% line coverage on every changed line, only pre-existing untouched branches remain uncovered
  • Full targeted regression run across 23 test files / 1127 tests spanning auth, orb ingest, reputation, blacklist, contributor caps, control-panel access boundary, and MCP — all green
  • Per updated instruction, the full npm run test:ci / npm audit were not run locally — left to CI

Note

One pre-existing, unrelated test failure was observed after rebasing onto the now-updated main (api.test.ts > serves installation repair diagnostics and refreshes installation health, failing on required-webhook-event health checks in src/github/app.ts/backfill.ts — code this PR never touches). It reproduces identically without this branch's changes and appears tied to the same root cause as the already-flagged #9169 regression.

…r trusted request bodies (#9121, #9125, #9126, #9131)

Four contributor-scoped controls trusted a mutable, attacker-influenced
identity signal instead of a stable, authenticated one:

- The risk-control ingest path trusted a body-supplied instance_id
  against a fleet-wide shared token, letting any fleet member plant or
  retract another instance's published guarantee. Ingest now requires
  a per-instance credential minted at registration, stores guarantees
  per-instance (aggregated at read time), and only an explicit null
  retracts an arm.
- The contributor blacklist, PR/issue open-item caps keyed on the
  renameable GitHub login alone. pull_requests/issues now persist the
  author's immutable id, and matching is id-when-present union login,
  so a rename can no longer clear a ban or a cap.
- Fleet-operator trust keyed on login alone, so a released-and-
  reregistered handle could grant operator access. ADMIN_GITHUB_IDS
  binds trust to the session's immutable id when configured, with a
  behavior-preserving login-only fallback.
- submitter_stats counted webhook passes, not submissions: no
  idempotency key meant every re-gate (or a third party's review
  comment) bumped a rival's counter. Recording is now idempotent per
  (project, submitter, pull_number, outcome) and the burst signal reads
  a decaying window instead of an all-time total.
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
loopover-ui c9585cf Commit Preview URL

Branch Preview URL
Jul 27 2026, 05:48 AM

@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored
JSONbored merged commit 6860b26 into main Jul 27, 2026
5 of 7 checks passed
@JSONbored
JSONbored deleted the fix/identity-binding-9121-9125-9126-9131 branch July 27, 2026 05:48
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

❌ 7 Tests Failed:

Tests completed Failed Passed Skipped
22479 7 22472 21
View the top 3 failed test(s) by shortest run time
test/unit/setup-wizard-docs-parity.test.ts > self-host GitHub App manifest <-> docs parity (#2542) > the docs page's events sentence names every buildManifest default_event, and only those
Stack Traces | 0.0158s run time
AssertionError: expected [ 'check run', 'check suite', …(5) ] to deeply equal [ 'check run', 'check suite', …(10) ]

- Expected
+ Received

  [
    "check run",
    "check suite",
-   "deployment status",
-   "issue comment",
    "issues",
    "pull request",
    "pull request review",
-   "pull request review thread",
    "push",
-   "repository",
    "status",
-   "workflow run",
  ]

 ❯ test/unit/setup-wizard-docs-parity.test.ts:65:31
test/unit/backfill.test.ts > GitHub backfill > refreshes installation health from live GitHub App metadata
Stack Traces | 0.0384s run time
AssertionError: expected [ { installationId: 123, …(19) } ] to deeply equal ArrayContaining{…}

- Expected
+ Received

- ArrayContaining [
-   ObjectContaining {
+ [
+   {
      "accountLogin": "JSONbored",
+     "authMode": "local",
+     "checkedAt": "2026-07-27T05:48:35.581Z",
+     "errorSummary": undefined,
+     "eventRemediation": [
+       {
+         "action": "No change needed.",
+         "event": "issues",
+         "ok": true,
+       },
+       {
+         "action": "No change needed.",
+         "event": "issue_comment",
+         "ok": true,
+       },
+       {
+         "action": "No change needed.",
+         "event": "pull_request",
+         "ok": true,
+       },
+       {
+         "action": "Subscribe to the pull_request_review webhook event.",
+         "event": "pull_request_review",
+         "ok": false,
+       },
+       {
+         "action": "No change needed.",
+         "event": "repository",
+         "ok": true,
+       },
+       {
+         "action": "Subscribe to the check_run webhook event.",
+         "event": "check_run",
+         "ok": false,
+       },
+       {
+         "action": "Subscribe to the check_suite webhook event.",
+         "event": "check_suite",
+         "ok": false,
+       },
+     ],
+     "events": [
+       "issues",
+       "issue_comment",
+       "pull_request",
+       "repository",
+       "installation_repositories",
+     ],
      "installationId": 123,
-     "missingEvents": [],
+     "installedReposCount": 0,
+     "missingEvents": [
+       "pull_request_review",
+       "check_run",
+       "check_suite",
+     ],
      "missingPermissions": [],
-     "status": "healthy",
+     "optionalPermissions": {
+       "checks": "write",
+     },
+     "optionalVisibleEvents": [
+       "installation_target",
+       "installation_repositories",
+     ],
+     "permissionRemediation": [
+       {
+         "action": "No change needed.",
+         "currentAccess": "read",
+         "ok": true,
+         "permission": "metadata",
+         "requiredAccess": "read",
+       },
+       {
+         "action": "No change needed.",
+         "currentAccess": "write",
+         "ok": true,
+         "permission": "pull_requests",
+         "requiredAccess": "read",
+       },
+       {
+         "action": "No change needed.",
+         "currentAccess": "write",
+         "ok": true,
+         "permission": "issues",
+         "requiredAccess": "write",
+       },
+     ],
+     "permissions": {
+       "checks": "write",
+       "issues": "write",
+       "metadata": "read",
+       "pull_requests": "write",
+     },
+     "registeredInstalledCount": 0,
+     "repairSteps": [
+       "Update the GitHub App permissions and subscribed events.",
+       "Approve the changed permissions or reinstall the app on the target account.",
+       "Run refresh-installation-health after GitHub sends the updated installation payload.",
+       "Recheck /v1/readiness and this installation health endpoint.",
+     ],
+     "repositorySelection": "selected",
+     "requiredEvents": [
+       "issues",
+       "issue_comment",
+       "pull_request",
+       "pull_request_review",
+       "repository",
+       "check_run",
+       "check_suite",
+     ],
+     "requiredPermissions": {
+       "issues": "write",
+       "metadata": "read",
+       "pull_requests": "read",
+     },
+     "status": "needs_attention",
    },
  ]

 ❯ test/unit/backfill.test.ts:1369:37
test/unit/queue-4.test.ts > queue processors > debounces noisy PR events without publishing public surfaces
Stack Traces | 0.133s run time
AssertionError: expected 3 to be +0 // Object.is equality

- Expected
+ Received

- 0
+ 3

 ❯ test/unit/queue-4.test.ts:2745:25
test/integration/api.test.ts > api routes > serves installation repair diagnostics and refreshes installation health
Stack Traces | 0.189s run time
AssertionError: expected { …(12) } to match object { refreshed: true, …(2) }
(37 matching properties omitted from actual)

- Expected
+ Received

@@ -1,10 +1,14 @@
  {
    "installation": {
-     "missingEvents": [],
+     "missingEvents": [
+       "pull_request_review",
+       "check_run",
+       "check_suite",
+     ],
      "missingPermissions": [],
-     "status": "healthy",
+     "status": "needs_attention",
    },
    "refreshed": true,
    "requiredPermissions": {
      "checks": "write",
      "issues": "write",

 ❯ test/integration/api.test.ts:2364:35
test/unit/backfill.test.ts > GitHub backfill > uses installation source for queued segment jobs and sparse live installation fallback metadata
Stack Traces | 0.198s run time
AssertionError: expected [ { installationId: 123, …(19) } ] to deeply equal ArrayContaining{…}

- Expected
+ Received

- ArrayContaining [
-   ObjectContaining {
+ [
+   {
      "accountLogin": "JSONbored",
+     "authMode": "local",
+     "checkedAt": "2026-07-27T05:48:41.202Z",
+     "errorSummary": undefined,
+     "eventRemediation": [
+       {
+         "action": "Subscribe to the issues webhook event.",
+         "event": "issues",
+         "ok": false,
+       },
+       {
+         "action": "Subscribe to the issue_comment webhook event.",
+         "event": "issue_comment",
+         "ok": false,
+       },
+       {
+         "action": "Subscribe to the pull_request webhook event.",
+         "event": "pull_request",
+         "ok": false,
+       },
+       {
+         "action": "Subscribe to the pull_request_review webhook event.",
+         "event": "pull_request_review",
+         "ok": false,
+       },
+       {
+         "action": "Subscribe to the repository webhook event.",
+         "event": "repository",
+         "ok": false,
+       },
+       {
+         "action": "Subscribe to the check_run webhook event.",
+         "event": "check_run",
+         "ok": false,
+       },
+       {
+         "action": "Subscribe to the check_suite webhook event.",
+         "event": "check_suite",
+         "ok": false,
+       },
+     ],
      "events": [],
      "installationId": 123,
+     "installedReposCount": 1,
      "missingEvents": [
        "issues",
        "issue_comment",
        "pull_request",
+       "pull_request_review",
        "repository",
+       "check_run",
+       "check_suite",
      ],
      "missingPermissions": [
        "metadata",
        "pull_requests",
        "issues",
      ],
+     "optionalPermissions": {
+       "checks": "write",
+     },
+     "optionalVisibleEvents": [
+       "installation_target",
+       "installation_repositories",
+     ],
+     "permissionRemediation": [
+       {
+         "action": "Set repository permission metadata to read.",
+         "currentAccess": "missing",
+         "ok": false,
+         "permission": "metadata",
+         "requiredAccess": "read",
+       },
+       {
+         "action": "Set repository permission pull_requests to read.",
+         "currentAccess": "missing",
+         "ok": false,
+         "permission": "pull_requests",
+         "requiredAccess": "read",
+       },
+       {
+         "action": "Set repository permission issues to write.",
+         "currentAccess": "missing",
+         "ok": false,
+         "permission": "issues",
+         "requiredAccess": "write",
+       },
+     ],
      "permissions": {},
+     "registeredInstalledCount": 1,
+     "repairSteps": [
+       "Update the GitHub App permissions and subscribed events.",
+       "Approve the changed permissions or reinstall the app on the target account.",
+       "Run refresh-installation-health after GitHub sends the updated installation payload.",
+       "Recheck /v1/readiness and this installation health endpoint.",
+     ],
      "repositorySelection": "selected",
+     "requiredEvents": [
+       "issues",
+       "issue_comment",
+       "pull_request",
+       "pull_request_review",
+       "repository",
+       "check_run",
+       "check_suite",
+     ],
+     "requiredPermissions": {
+       "issues": "write",
+       "metadata": "read",
+       "pull_requests": "read",
+     },
      "status": "needs_attention",
    },
  ]

 ❯ test/unit/backfill.test.ts:4724:34
test/unit/backfill.test.ts > GitHub backfill > reports installation health from stored permissions and events
Stack Traces | 0.242s run time
AssertionError: expected { installationId: 123, …(19) } to match object { status: 'needs_attention', …(3) }
(22 matching properties omitted from actual)

- Expected
+ Received

@@ -1,10 +1,13 @@
  {
    "missingEvents": [
      "issues",
      "issue_comment",
+     "pull_request_review",
      "repository",
+     "check_run",
+     "check_suite",
    ],
    "missingPermissions": [
      "pull_requests",
      "issues",
    ],

 ❯ test/unit/backfill.test.ts:686:37
test/unit/check-branding-drift-script.test.ts > check-branding-drift script (real repo state) > the committed baseline matches the real current repo state (regression guard)
Stack Traces | 0.394s run time
Error: Command failed: .../hostedtoolcache/node/22.23.1.../x64/bin/node --experimental-strip-types scripts/check-branding-drift.ts
Branding-drift check found 1 issue(s):
src/review/predicted-gate-agreement.ts: "gittensory" mentions increased from 0 to 1 -- looks like new branding drift, not an intentional historical reference. If it genuinely belongs (e.g. a permanent Sentry ticket ID or a stable comment-marker already posted to live PRs), run `npm run branding-drift:update` and commit the regenerated baseline.

 ❯ test/unit/check-branding-drift-script.test.ts:143:20

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { status: 1, signal: null, output: [ null, '', 'Branding-drift check found 1 issue(s):\nsrc/review/predicted-gate-agreement.ts: "gittensory" mentions increased from 0 to 1 -- looks like new branding drift, not an intentional historical reference. If it genuinely belongs (e.g. a permanent Sentry ticket ID or a stable comment-marker already posted to live PRs), run `npm run branding-drift:update` and commit the regenerated baseline.\n' ], pid: 20766, stdout: '', stderr: 'Branding-drift check found 1 issue(s):\nsrc/review/predicted-gate-agreement.ts: "gittensory" mentions increased from 0 to 1 -- looks like new branding drift, not an intentional historical reference. If it genuinely belongs (e.g. a permanent Sentry ticket ID or a stable comment-marker already posted to live PRs), run `npm run branding-drift:update` and commit the regenerated baseline.\n' }

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

JSONbored added a commit that referenced this pull request Jul 27, 2026
…ction, pool decisionAccuracy (#9068)

orb_signals ingest stored any object-shaped risk_control payload verbatim, so a
registered instance with a stale or misconfigured alpha (or an outright refused
calibration) could reach the public guarantee unchecked. handleOrbIngest now runs
the payload through validateCalibrationPayload (status === "calibrated", alpha/
lambda/coverage in range, nAtLambda clearing the zero-error floor for its own
alpha/delta) before it reaches orb_risk_control_arms at all.

gamingPatternFlags compared each eligible instance against the fleet median, which
is structurally unfireable below 3 eligible instances (an instance IS the median at
n=1) and can never flag "low reversal" once the fleet's own reversal-rate median is
exactly 0 (a common, healthy-fleet case) since a fraction of zero can never be
undercut. computeFleetAnalytics now gates detection on eligible.length >= 3
(surfaced via the new gamingDetectionEligible field) and falls back to an absolute
reversal-rate floor when the fleet median is zero.

fleet.decisionAccuracy published the per-instance MEDIAN while accuracyCiPct
(public-stats.ts) is a Wilson interval over the POOLED counts — different
estimands that only coincide at equal per-instance volumes. fleet.decisionAccuracy
now publishes the pooled proportion directly (the same population the interval
describes); the per-instance median survives as the new decisionAccuracyMedian
diagnostic field.

The underlying per-instance last-writer-wins fleet-key bug this issue also
describes was already fixed by #9177 (orb_risk_control_arms is keyed per
instance_id/arm and public-stats already aggregates across registered instances at
read time) — this change covers the remaining validation-before-publish and
detection-floor gaps.
JSONbored added a commit that referenced this pull request Jul 27, 2026
…ntee's fleet integrity (#9228)

* fix(review): split threshold-refusal outcomes and Bonferroni-correct calibration delta (#9048, #9066)

calibrateActThreshold returned "insufficient_labels" from two branches with different
"have" semantics: a genuine label shortfall (have = total pairs) and a residual
high-confidence stratum too small to certify despite ample total labels (have = that
stratum's size). The latter now returns a distinct "no_certifiable_threshold" status
carrying totalPairs/bestN/bestLambda/bestUpperBound, rendered through its own message
and a distinct risk_control_no_certifiable_threshold audit event so the label
burn-down no longer conflates "needs labels" with "needs a better error rate".

Separately, the ascending-lambda scan reported whichever of K observed-confidence
candidates passed first without correcting for testing K of them — the advertised
1-delta confidence overstated what the scan actually delivered. Each candidate is now
tested at a Bonferroni-split delta/K, so the certified lambda is valid at the full,
originally-advertised delta regardless of which candidate passes. Chosen over a true
fixed-sequence rewrite as the smaller, safer diff against the existing ascending scan.

Also adds AND dr2.action = dal.verdict to the calibration join (latent-risk hardening:
today's data is not mis-joined, but a later HOLD/MERGE record on the same PR could
otherwise shadow the acted CLOSE record a label adjudicates) and tags each calibration
pair's provenance (backfilled vs live, via the backfill's configDigest sentinel) so a
published guarantee can later say how much of its evidence is reconstructed history.

* fix(orb): validate risk-control payloads at ingest, floor gaming detection, pool decisionAccuracy (#9068)

orb_signals ingest stored any object-shaped risk_control payload verbatim, so a
registered instance with a stale or misconfigured alpha (or an outright refused
calibration) could reach the public guarantee unchecked. handleOrbIngest now runs
the payload through validateCalibrationPayload (status === "calibrated", alpha/
lambda/coverage in range, nAtLambda clearing the zero-error floor for its own
alpha/delta) before it reaches orb_risk_control_arms at all.

gamingPatternFlags compared each eligible instance against the fleet median, which
is structurally unfireable below 3 eligible instances (an instance IS the median at
n=1) and can never flag "low reversal" once the fleet's own reversal-rate median is
exactly 0 (a common, healthy-fleet case) since a fraction of zero can never be
undercut. computeFleetAnalytics now gates detection on eligible.length >= 3
(surfaced via the new gamingDetectionEligible field) and falls back to an absolute
reversal-rate floor when the fleet median is zero.

fleet.decisionAccuracy published the per-instance MEDIAN while accuracyCiPct
(public-stats.ts) is a Wilson interval over the POOLED counts — different
estimands that only coincide at equal per-instance volumes. fleet.decisionAccuracy
now publishes the pooled proportion directly (the same population the interval
describes); the per-instance median survives as the new decisionAccuracyMedian
diagnostic field.

The underlying per-instance last-writer-wins fleet-key bug this issue also
describes was already fixed by #9177 (orb_risk_control_arms is keyed per
instance_id/arm and public-stats already aggregates across registered instances at
read time) — this change covers the remaining validation-before-publish and
detection-floor gaps.

* fix(review): disambiguate the published guarantee's coverage and validate it before serving (#9050)

readGuarantee published "coveragePct" as if it were a share of all closes, adjacent
to a different fleetAccuracy.coveragePct that IS a share of all decided signals --
one word, two denominators. The guarantee's own field is actually the share of the
arm's AI-JUDGED sub-population the threshold covers (loadCalibrationPairs can only
join a confidence to decisions an AI-judgment blocker ran on, a minority of real
closes). Renamed to aiJudgedCoveragePct and the homepage string now names the
sub-population explicitly instead of leaving a bare percentage next to its sibling.

readGuarantee also now re-validates every stored orb_risk_control_arms row through
risk-control.ts's validateCalibrationPayload (defense in depth alongside the
ingest-side check from the companion #9068 fix) and walks all registered rows for
an arm in nAtLambda-descending order instead of trusting only the top one, so a
single malformed or stale peer can no longer hide a good row behind it.

Each calibration pair now carries whether it's backfilled (the 2026-07
calibration-corpus backfill's configDigest sentinel) or live; calibrateActThreshold
surfaces the split as backfilledPairs, and the public guarantee renders it as
backfilledPct so a guarantee resting mostly on reconstructed history says so.

Regenerated apps/loopover-ui/public/openapi.json for the schema rename/nullability.

* style(ui): run prettier over the guarantee coverage rename/test additions

npm run ui:lint's format:check caught unformatted lines from the #9050 fleetAccuracy.guaranteed rename in proof-of-power-stats-model.ts and its new test.
This was referenced Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment