fix(registry): an installer who is also an instance admin gets the instance-wide scope (TASK-060) - #1752
Merged
Conversation
…stance-wide scope (TASK-060) The follow-up Wren 69565(b) asked for, filed on Sam 69582 when #1746 merged: authority is a role, not a coincidence of which row the caller created. #1746 scoped the fan-out as `isInstaller ? own rows : all rows`, which gave an instance admin who happened to be the row's installer the NARROWER scope — so the same admin got two different reaches depending on which of two rows they opened. The scope now follows the role: `isInstanceAdmin ? all rows : own rows`. An installer who is not an admin is unchanged, and that half is pinned by the existing test ("fans the fan-out out only to the rows the caller installed"). `isInstanceAdmin` is now computed on every PATCH rather than only on the non-installer path, and deliberately: it is the input to the scope decision, and deriving it only when the caller was not the installer would have made the scope depend on how they authenticated (a session token carries no `role`, an API token does), which is the same inconsistency one layer down. One extra indexed user read on a PATCH buys a scope that does not vary by auth path. Two sibling fixtures needed a `User` mock because the handler now always asks the admin question — they are installers of both rows and not admins, so their intent is preserved exactly, no assertion changed. Tests: new case in registry.agent-config-installer-gate.test.js — installer of pod-1 who is also an instance admin, pod-2's row installed by someone else: updatedPods 2, both rows saved, AgentProfile.updateMany scoped to both pods. 28 registry route suites / 103 tests pass, backend .ts lint 0 errors, tsc:check clean.
This was referenced Sep 19, 2026
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
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.
What
The follow-up to #1746 that Wren 69565(b) asked for and Sam 69582 filed as its own row: authority is a role, not a coincidence of which row the caller created.
#1746 gated the PATCH on installer-or-admin but scoped the fan-out as
isInstaller ? own rows : all rows. That gave an instance admin who happened to be the row's installer the narrower scope, so the same admin got two different reaches depending on which of two rows they opened. The scope now follows the role:An installer who is not an admin is unchanged — that half is pinned by the existing test
fans the fan-out out only to the rows the caller installed.One deliberate side effect
isInstanceAdminis now computed on every PATCH rather than only on the non-installer path. That is the point: it is an input to the scope decision, and deriving it only when the caller was not the installer would make the scope depend on how they authenticated — a session token carries norole, an API token does — which is the same inconsistency one layer down. One extra indexed user read on a PATCH buys a scope that does not vary by auth path.Two sibling fixtures (
registry.patch-shared-settings,registry.tool-policy) gained aUsermock because the handler now always asks the admin question. They are installers of both rows and not admins, so their intent is preserved exactly and no assertion changed.Tests
New case in
registry.agent-config-installer-gate.test.js: installer of pod-1 who is also an instance admin, pod-2's row installed by someone else →updatedPods: 2, both rows saved, andAgentProfile.updateManyscoped to{$in: ['pod-1', 'pod-2']}..tslint: 0 errors (1206 pre-existing warnings)tsc:checkcleanNot run: the full backend unit suite (a 10-line change in one handler plus one test, and CI runs it). Refs TASK-060.