fix(orb): installation/permission/config integrity — suspension backfill, broker identity, numeric env validation, migration content hash#9222
Merged
JSONbored merged 5 commits intoJul 27, 2026
Conversation
…consent (#9151) listOrbAppInstallations now parses suspended_at from GitHub's installation list, and backfillOrbInstallations writes it through instead of hardcoding NULL on every upsert. Previously a suspension recorded by the installation.suspend webhook was silently cleared by the next backfill, making the broker's suspended_at eligibility check always-passing. removed_at stays cleared: GitHub's installations list never reports an actually-uninstalled App, so every install this loop sees is by definition still installed.
…aborator role (#9152) mintInstallationToken now throws when a brokered token's installationId is non-zero and differs from the requested install, instead of caching and returning it under the caller's id — closing a cross-tenant token-serving path a stale caller-side installationId (e.g. after an App uninstall/reinstall) could reach. The permissions write is no longer gated on an installationId match that a legacy zero-default reply could never satisfy. getRepositoryCollaboratorPermission now prefers the collaborator-permission endpoint's role_name over its coarse permission field, since permission only ever resolves to admin/write/read/none — collapsing Maintain into "write" and Triage into "read" and making the "maintain" tier unreachable at both isPerTenantAdmin and resolveRealRepoPermissionAssociation.
…a bad value NaN through (#9157) preflightEnv now hard-fails boot when CRON_INTERVAL_MS, PORT, or GITHUB_CACHE_TTL_SECONDS is set to a non-plain-integer or out-of-range value, via a new positiveInteger check. Previously a bare Number(process.env.X ?? default) silently turned a unit-suffixed or malformed value into NaN, which setTimeout/setInterval coerce to a 1ms delay (spinning the cron at ~1000 ticks/second) or which a `> 0` gate always fails (silently disabling the GitHub response cache). server.ts's three call sites now also read through parsePositiveIntEnv (queue-common.ts) as runtime defense-in-depth, floored/clamped with a warning instead of coercing to NaN. CRON_INTERVAL_MS=0 is explicitly NOT treated as "disable the cron" — there is no supported way to run this entrypoint without its maintenance cron.
…a a content hash (#9164) The self-host migration ledger (_selfhost_migrations) now records each applied file's content_sha256 alongside its name. On every boot, runSelfHostMigrations recomputes the current on-disk hash of every already-applied file and compares it to the recorded one: a mismatch fails boot loudly (logged, then thrown) instead of the prior behavior, where the ledger's filename-only key made an in-place edit after apply invisible forever. A pre-#9164 row with no stored hash is backfilled to its current content as the baseline, since the original apply-time content can't be recovered. No down-migration path is added — this repo's migrations remain forward-only.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…ations npm run selfhost:env-reference was not re-run after CRON_INTERVAL_MS/PORT/ GITHUB_CACHE_TTL_SECONDS validation moved into preflight.ts (#9157).
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | d4cffca | Commit Preview URL Branch Preview URL |
Jul 27 2026, 07:51 AM |
Bundle ReportChanges will increase total bundle size by 36 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
JSONbored
deleted the
fix/9151-9152-9157-9164-install-permission-integrity
branch
July 27, 2026 07:56
loopover-orb
Bot
added
the
gittensor:bug
❌ 4 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
backfillOrbInstallationsunconditionally clearedsuspended_attoNULLon every upsert, andlistOrbAppInstallationsnever parsed GitHub'ssuspended_atfield in the first place. A suspension recorded by theinstallation.suspendwebhook was silently erased by the next backfill, making the broker'sinstall.suspended_at !== nulleligibility check structurally always-passing.getRepositoryCollaboratorPermissionread GitHub's coarsepermissionfield (admin/write/read/none only), making the"maintain"tier permanently unreachable at both call sites that check for it.CRON_INTERVAL_MS,PORT, andGITHUB_CACHE_TTL_SECONDSwere all read with a bareNumber(process.env.X ?? default). A malformed value (unit suffix, numeric separator, garbage) becomesNaN, whichsetTimeout/setIntervalcoerce to a 1ms delay (runaway cron) or which silently fails a> 0gate (disabled response cache) — with no boot-time signal either way._selfhost_migrations) recorded only a filename, so a migration file edited in place after being applied was undetectable: the running DB and the repo's declared schema could silently diverge with no signal at boot, in/health, or inpreflight.ts.Changes
src/orb/app-auth.ts/src/orb/installations.ts: parse and write throughsuspended_atin the backfill instead of hardcodingNULL.removed_atstill clears — GitHub's installations list never reports an actually-uninstalled App, so every install this loop sees is by definition still installed (documented inline).src/github/app.ts:mintInstallationToken's broker branch now throws when the broker's claimedinstallationIdis non-zero and differs from the requested one, instead of silently caching/returning that token under the caller's id. The permissions-persist step is no longer gated on an installationId match that a legacy zero-default reply could never satisfy (a second latent bug the same code shape hid).getRepositoryCollaboratorPermissionnow prefersrole_nameoverpermission, falling back topermissiononly whenrole_nameis absent.src/selfhost/preflight.ts: newpositiveIntegerhelper;CRON_INTERVAL_MS,PORT, andGITHUB_CACHE_TTL_SECONDSare validated at boot and fail loudly (not silently coerced) when set to a non-integer, unit-suffixed, or out-of-range value.src/selfhost/cron-alignment.tsgets a sharedCRON_INTERVAL_MIN_MSfloor constant.src/server.ts's three call sites now read through the existingparsePositiveIntEnv(queue-common.ts) as runtime defense-in-depth (floors/clamps with a warning instead of coercing to NaN).src/selfhost/migrate.ts:_selfhost_migrationsgains acontent_sha256column, populated on apply and checked against the current on-disk hash of every already-applied file on every boot. A mismatch logs loudly and fails boot (thrown) instead of being silently skipped. A pre-existing row with no stored hash is backfilled to its current content as the baseline.Judgment calls
migrations/*.sqlfile; stated explicitly in the function's doc comment per the issue's own ask.migrations/NNNN_*.sqlfile for orb(selfhost): the migration ledger records only a filename — a migration edited after it was applied is undetectable, and a half-applied file is recorded as complete #9164'scontent_sha256column:_selfhost_migrationsis a purely self-host runtime table, created and evolved entirely insidemigrate.ts(CREATE TABLE IF NOT EXISTS/ALTER TABLE ... ADD COLUMN, tolerating "already exists" like the rest of the file) — confirmed via search, it has never had a numbered migration or aschema.tsentry, since Cloudflare D1 never runs this code path. Adding one would create an unused table in the cloud DB and duplicate the table's actual, working self-managed-schema mechanism.CRON_INTERVAL_MS=0is explicitly rejected, not treated as "disable the cron" — there's no supported way to run this entrypoint without its maintenance cron (sweep dispatch, ops-alerts, reconciliation, registry refresh), so 0 is just another invalid value.PORT1–65535,GITHUB_CACHE_TTL_SECONDS0–86400,CRON_INTERVAL_MS10s–24h) are reasonable defaults not spelled out in the issue.Deferrals / already-fixed check
All four issues were reproduced against current
mainand none were already fixed by a recent merge.Validation
npx tsc --noEmit -p tsconfig.json --incremental false— cleantest/unit/orb-app-auth.test.ts,test/integration/orb-installations.test.ts,test/unit/github-app.test.ts,test/unit/auth-per-repo-admin.test.ts,test/unit/selfhost-preflight.test.ts,test/unit/selfhost-cron-alignment.test.ts,test/unit/selfhost-migrate.test.ts— 166 testsnpm run test:changed— 449 files / 11025 tests passed; the only 4 failures (orb-ingest.test.ts,orb-onboarding.test.ts,orb-relay.test.ts×2) are pre-existing onorigin/mainwithout this branch's changes (verified viagit stash), unrelated to this PRnpm run test:coverage(unsharded) — not run locally per task instructions; CI'scodecov/patchwill reportnpm run ui:*— no UI changes in this PRSafety
Closes #9151
Closes #9152
Closes #9157
Closes #9164