fix: installation-scoped tenant isolation for hosted multi-tenant app - #1098
Closed
Helios531 wants to merge 6 commits into
Closed
fix: installation-scoped tenant isolation for hosted multi-tenant app#1098Helios531 wants to merge 6 commits into
Helios531 wants to merge 6 commits into
Conversation
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Contributor
Author
|
@JSONbored Could you review my PR? |
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review — closed
🛑 Closed — AI reviewers agree on a likely critical defect: src/db/repositories.ts: markInstallationDeleted and markRepositoriesRemovedFromInstallation set installationId: sql`null` even though the column is defined as NOT NULL with a default of 0, causing a constraint violation. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate. Review summary Blockers
<details>
Why this is blocked
Nits — 1 non-blocking
Review context
Contributor next steps
Signal definitions
Review detailsGenerated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative. The PR adds installation‑scoped isolation for repository settings, BYOK keys, and guardrail config, introduces a migration, updates DB schema, and adds subnet claim validation via metagraphed. Core logic correctly resolves the active installation ID and scopes queries, and new tests cover the isolation behavior. However, the deletion helpers now attempt to write NULL into a NOT NULL column, which will cause runtime DB errors. Blockers
<details>
🟩 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.
|
❌ 14 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
JSONbored
added a commit
that referenced
this pull request
Jun 23, 2026
…(guard blocks merge, not rejection) (#1106) #1090 gated willClose on !guardrailHit, so a rejected CONTRIBUTOR PR touching a guarded path was HELD instead of closed (e.g. gittensory #1098: rejected, touches src/review|services|signals → stayed open with a 'closing' message). That contradicts the spec: 'guarded + would-merge → hold; otherwise → closure.' The hard-guardrail exists to stop auto-MERGING/APPROVING crucial-path changes without owner review (canMerge + approve still gate on !guardrailHit). It must NOT keep a rejected PR open — closing rejects bad changes and merges nothing, so it is always safe. willClose no longer checks guardrailHit; owner/automation PRs are still never closed (isContributor gates it); GOOD-but-guarded PRs still fall through to the owner (held). Test flipped: a failing contributor PR on a guarded path now CLOSES.
JSONbored
reviewed
Jun 23, 2026
JSONbored
left a comment
Owner
There was a problem hiding this comment.
The issue is already assigned to maintainer-only and is not worth any gittensor points. Closing.
Owner
|
T |
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
2 participants
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
This change fixes multi-tenant isolation for hosted app usage.
Before this fix, some tenant-sensitive state could be resolved too broadly:
This patch makes those paths installation-scoped so hosted tenants are isolated from each other, while preserving a safe legacy/self-host fallback path.
Related Issues
Change Type
What Changed
repository_settingsrepository_ai_keysReal Behavior Proof
Before
After
repo_full_name + installation_idrepo_full_name + installation_idinstallation:{installationId}:{owner/repo}installation:{installationId}:{slug}owner/reposlugThis prevents hosted tenant cross-bleed for private runtime config and secrets.
Checklist