Skip to content

feat(api): tenant self-service for installation health/repair - #8006

Closed
rsnetworkinginc wants to merge 1 commit into
JSONbored:mainfrom
rsnetworkinginc:tenant-installation-selfservice-7661
Closed

feat(api): tenant self-service for installation health/repair#8006
rsnetworkinginc wants to merge 1 commit into
JSONbored:mainfrom
rsnetworkinginc:tenant-installation-selfservice-7661

Conversation

@rsnetworkinginc

Copy link
Copy Markdown
Contributor

Summary

The installation health/repair routes — GET /v1/installations, GET /v1/installations/:id/health, GET /v1/installations/:id/repair, and POST /v1/installations/:id/repair/refresh (src/api/routes.ts) — carry no inline role/ownership check. They are reachable only by the global fleet operator, because canSessionAccessPath falls through to the operator gate for any path not on its explicit allowlist. A hosted tenant therefore has no self-service way to see or repair even their own installation's health.

This adds a tenant-scoped equivalent under /v1/app/installations*, reusing /v1/app/maintainer-dashboard's exact scoping mechanism (loadControlPanelAccessScope in src/services/control-panel-roles.ts) rather than inventing a new auth path. An operator (or static service identity) still sees the whole fleet; a non-operator session is limited to installations under their own account or maintained repos, so tenant A can never read or trigger repair on tenant B's installation. No bulk/cross-repo controls are added (that is #7676, out of scope).

Closes #7661

What changed

  • src/api/routes.ts (+78): adds four tenant-scoped sibling routes next to the operator-only installation routes:
    • GET /v1/app/installations — lists only the caller's in-scope installations and health.
    • GET /v1/app/installations/:id/health — the caller's own installation health (403 forbidden_installation otherwise).
    • GET /v1/app/installations/:id/repair — repair diagnostics for the caller's own installation.
    • POST /v1/app/installations/:id/repair/refresh — refreshes the caller's own installation; ownership is enforced before the refresh side effect so a tenant can never trigger repair on an installation they don't own.
    • Two small helpers: resolveAppInstallationScope (mirrors the maintainer-dashboard role gate + loadControlPanelAccessScope step, returning scope === null for operators/service identities) and installationRecordInScope (the exact installation-id / account-login match maintainer-dashboard applies). The unauthenticated guard reuses the file's established /* v8 ignore next */ pattern, since the global protected-route middleware already rejects unauthenticated private requests before the handler.
  • test/integration/app-installations-selfservice.test.ts (+249, new): a dedicated in-process integration test driving every route and every added branch via app.request.

Tests

New dedicated test file exercises all four routes end-to-end in-process, covering every added branch and error path: operator/api-token whole-fleet access, tenant-scoped list, own-installation reads (both the installation-id and account-login scope arms), the ownership-denied 403 forbidden_installation path (tenant A → tenant B), the insufficient_role gate on every route, invalid-id 400, not-found 404, the refresh installation_not_found path, and a successful own-installation refresh. Measured coverage the codecov way (vitest --coverage --coverage.reporter=json) and cross-checked every added line in the diff against coverage-final.json: 0 uncovered statements and 0 uncovered branches on the changed source.

Verified locally on current main: typecheck/engine build/git diff --check clean, 7 new tests pass (77 pass across the touched integration + openapi suites), 100% patch coverage — src/api/routes.ts: 0 uncovered statement lines, 0 uncovered branch lines on the added diff (src/services/control-panel-roles.ts unchanged).

@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 22, 2026
@loopover-orb

loopover-orb Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-22 10:39:26 UTC

2 files · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This adds four tenant-scoped installation health/repair routes under /v1/app/installations*, reusing the existing loadControlPanelAccessScope mechanism from /v1/app/maintainer-dashboard rather than inventing new auth. The scoping helper (installationRecordInScope) correctly matches by installation id or account login, ownership is checked before any repair side effect in the POST refresh route, and the accompanying test file exercises every branch (401/403/400/404/200) including the operator whole-fleet case and the orphaned-health-row account-login-scope arm. The implementation is a straightforward, well-tested sibling of an existing pattern with no logic defects visible in the diff.

Nits — 5 non-blocking
  • The 'leaked secret' flagged in the external brief at test/integration/app-installations-selfservice.test.ts:248 is a runtime-generated RSA keypair (crypto.subtle.generateKey) used only for signing a mock JWT in the test, not an actual credential — safe to ignore.
  • src/api/routes.ts: the new GET /v1/app/installations list route fetches the entire fleet via listInstallations/listInstallationHealth and filters in memory rather than scoping at the query level; acceptable if the existing operator route does the same, but worth confirming it won't become a bottleneck as fleet size grows.
  • Consider a short comment or shared constant for the repeated role array ['maintainer', 'owner', 'operator'] in resolveAppInstallationScope, since the same list appears in requireAppRole/requireRepoMaintainer call sites — minor DRY opportunity.
  • If the fleet is large, consider adding a scoped query path (filter by installationIds/accountLogins in SQL) instead of list-then-filter for GET /v1/app/installations, mirrored from the existing operator route if it already does this.
  • Extract the ['maintainer','owner','operator'] role array in resolveAppInstallationScope (src/api/routes.ts) into a shared constant if it's reused elsewhere.

Why this is blocked

  • Possible leaked secret in the diff (private_key_block): The PR diff matches secret pattern(s): private_key_block. Found at: test/integration/app-installations-selfservice.test.ts:248. A committed credential must be rotated and removed from the change before merge. — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Possible leaked secret in the diff (private_key_block): The PR diff matches secret pattern(s): private_key_block. Found at: test/integration/app-installations-selfservice.test.ts:248. A committed credential must be rotated and removed from the change before merge. — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7661
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 10 registered-repo PR(s), 2 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor rsnetworkinginc; Gittensor profile; 10 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The PR adds tenant-scoped `/v1/app/installations*` routes (list, health, repair, repair/refresh) that reuse the maintainer-dashboard's exact `loadControlPanelAccessScope` scoping via a new `resolveAppInstallationScope` helper, matching the issue's proposal precisely and explicitly excluding bulk controls (#7676) as required.

Review context
  • Author: rsnetworkinginc
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 10 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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.
🧪 Chat with LoopOver

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

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

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (Possible leaked secret in the diff (private_key_block)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 22, 2026
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.80%. Comparing base (2656eaa) to head (6f3e41c).
⚠️ Report is 12 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8006      +/-   ##
==========================================
- Coverage   91.92%   89.80%   -2.13%     
==========================================
  Files         741       95     -646     
  Lines       75957    24371   -51586     
  Branches    23048     4587   -18461     
==========================================
- Hits        69827    21887   -47940     
+ Misses       5037     2214    -2823     
+ Partials     1093      270     -823     
Flag Coverage Δ
shard-1 29.04% <100.00%> (-27.76%) ⬇️
shard-2 43.94% <8.69%> (-8.75%) ⬇️
shard-3 71.39% <8.69%> (+20.14%) ⬆️

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

Files with missing lines Coverage Δ
src/api/routes.ts 95.27% <100.00%> (+0.08%) ⬆️

... and 646 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tenant self-service for installation health/repair (currently operator-only)

1 participant