Skip to content

chore(auth): remove the fully-deleted old GitHub App's dead vars/secrets#5269

Merged
JSONbored merged 2 commits into
mainfrom
chore/remove-old-app-refs
Jul 12, 2026
Merged

chore(auth): remove the fully-deleted old GitHub App's dead vars/secrets#5269
JSONbored merged 2 commits into
mainfrom
chore/remove-old-app-refs

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • The old gittensory GitHub App (id 3824093) has been fully deleted (not just suspended). Removed its now-dead references:
    • GITHUB_APP_ID / GITHUB_APP_SLUG removed from wrangler.jsonc's vars.
    • GITHUB_APP_PRIVATE_KEY / GITHUB_WEBHOOK_SECRET Worker secrets already deleted directly (operational, not a repo diff).
    • Widened all 4 corresponding env.d.ts types to optional, so a self-hoster running their own App under these same var names still works without code changes.
  • Fixed two latent crash sites the widening surfaced: isOwnGitHubAppCheckRun and isOwnReviewThreadAuthor (src/github/backfill.ts) called env.GITHUB_APP_SLUG.trim() unguarded — would throw for any deployment (including a self-hoster) that leaves it unset. Both now degrade to "no match" gracefully, mirroring the existing defensive pattern used elsewhere in the codebase (self-authored.ts's normalizeGitHubSlug).
  • Fixed a real correctness bug this surfaced: the public, unauthenticated GET /v1/public/subnet-interface descriptor's install-app slug read GITHUB_APP_SLUG — which would have silently become /apps/undefined in the response once the var was removed. Replaced with a stable, hardcoded "gittensory-orb" (confirmed: that's the real, currently publicly-installable app), decoupled entirely from whatever credentials this Worker happens to hold for its own operational purposes — so a future credential change can't break this public descriptor again.
  • createAppJwt (src/github/app.ts) now validates both GITHUB_APP_PRIVATE_KEY and GITHUB_APP_ID upfront (was only checking the key before), with the retry/eviction logic downstream narrowed to a guaranteed-defined local instead of re-reading the now-optional env.GITHUB_APP_ID.

Validation

  • npm run typecheck
  • Updated/fixed the affected tests: test/unit/webhook.test.ts, test/integration/api.test.ts (test-local signWebhook helpers now guard against an unset secret with a clear error instead of a type error), test/unit/subnet-interface.test.ts + test/integration/subnet-interface.test.ts (updated to the new hardcoded slug).
  • npm run test:ci (full local gate, unsharded) — green.
  • Empirically confirmed (not just reasoned) that removing GITHUB_APP_ID/GITHUB_APP_SLUG from wrangler.jsonc introduces no new compile error beyond the two intentionally-fixed sites.

Safety

  • No secrets committed.
  • A self-hoster running their own App under GITHUB_APP_ID/GITHUB_APP_SLUG/etc. is unaffected — these vars are now optional, not removed from the type surface.
  • The /v1/public/subnet-interface slug change is a deliberate, confirmed decision (not an inferred one) — verified with the maintainer that gittensory-orb is the real, publicly installable app.

GITHUB_APP_ID/GITHUB_APP_SLUG removed from wrangler.jsonc; the matching
GITHUB_APP_PRIVATE_KEY/GITHUB_WEBHOOK_SECRET Worker secrets already
deleted directly (they're now orphaned -- the App itself is gone, not
just suspended). Widened the 4 corresponding env.d.ts types to optional
so a self-hoster running their own App under these names still works.

Fixed two latent crash sites this surfaced: isOwnGitHubAppCheckRun and
isOwnReviewThreadAuthor (src/github/backfill.ts) called
GITHUB_APP_SLUG.trim() unguarded -- would throw for any deployment
(including a self-hoster) that leaves it unset. Both now degrade to
"no match" instead.

The public /v1/public/subnet-interface descriptor's install-app slug
was reading this same var -- silently becoming "/apps/undefined" once
removed. Replaced with a stable, hardcoded "gittensory-orb" (confirmed:
that's the real, currently publicly-installable app), decoupled from
whatever credentials this Worker happens to hold for its own
operational purposes.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.37%. Comparing base (864c52d) to head (0b386b7).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5269   +/-   ##
=======================================
  Coverage   94.37%   94.37%           
=======================================
  Files         474      474           
  Lines       40114    40118    +4     
  Branches    14627    14629    +2     
=======================================
+ Hits        37856    37860    +4     
  Misses       1583     1583           
  Partials      675      675           
Flag Coverage Δ
shard-1 46.27% <53.84%> (-0.15%) ⬇️
shard-2 34.74% <69.23%> (+0.01%) ⬆️
shard-3 32.06% <46.15%> (-0.04%) ⬇️
shard-4 31.82% <61.53%> (-0.26%) ⬇️
shard-5 33.60% <76.92%> (-0.07%) ⬇️
shard-6 45.06% <61.53%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/api/routes.ts 94.30% <100.00%> (ø)
src/github/app.ts 97.89% <100.00%> (+0.02%) ⬆️
src/github/backfill.ts 97.21% <100.00%> (ø)
src/services/subnet-interface.ts 100.00% <100.00%> (ø)
src/utils/crypto.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 12, 2026
@loopover-orb

loopover-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-12 12:57:23 UTC

12 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI failing · unstable

🛑 Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

Review summary
This PR cleans up dead vars from a fully-deleted GitHub App, widens the corresponding env.d.ts types to optional, and fixes two real unguarded `.trim()` crash sites in backfill.ts that widening would otherwise surface. It also correctly decouples the public subnet-interface descriptor's install-app slug from the (now-optional) env var by hardcoding the real, stable, publicly-installable app slug — a genuine correctness fix since the old code would have silently rendered `/apps/undefined`. The `appId`/`privateKey` local-narrowing in `createAppJwt`/`mintInstallationToken` is defensible TypeScript narrowing given the upfront validation, and the crypto/webhook signature helpers were updated in lockstep with the now-optional secret type.

Nits — 5 non-blocking
  • The hardcoded `PUBLIC_GITHUB_APP_SLUG = "gittensory-orb"` in src/services/subnet-interface.ts has no test asserting it stays independent of `env.GITHUB_APP_SLUG` being set to something else — worth a quick regression test to lock in the decoupling.
  • codecov/patch is failing at 84.61% (target 99%); the `/* v8 ignore next */`-marked unreachable branch in src/github/app.ts is understandable but check the guard branches in `isOwnGitHubAppCheckRun`/`isOwnReviewThreadAuthor` for missing coverage on the unset/blank `GITHUB_APP_SLUG` arm.
  • The comment in wrangler.jsonc documents 2026-07-12 as the removal date inline in code, which will read oddly a year from now — consider moving date context to the PR/commit message only.
  • Add a unit test in src/services/subnet-interface.ts's test file that explicitly passes a differing appSlug-like env var to prove the descriptor's slug is now immune to it.
  • Confirm the codecov/patch failure isn't hiding an actually-untested branch in the two backfill.ts guard functions before merge.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

CI checks failing

  • codecov/patch — 84.61% of diff hit (target 99.00%)
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 44 registered-repo PR(s), 36 merged, 470 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 470 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 44 PR(s), 470 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 12, 2026
Closes the codecov/patch branch-coverage gap on the ?? "" fallback in
isOwnGitHubAppCheckRun and isOwnReviewThreadAuthor, now that the field
is optional.
@JSONbored
JSONbored merged commit d56f4ec into main Jul 12, 2026
17 checks passed
@JSONbored
JSONbored deleted the chore/remove-old-app-refs branch July 12, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant