Skip to content

fix(registry): an installer who is also an instance admin gets the instance-wide scope (TASK-060) - #1752

Merged
lilyshen0722 merged 1 commit into
mainfrom
kai/1746-role-scoped-fanout
Sep 19, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
kai/1746-role-scoped-fanout

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

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:

const installationsToUpdate = isInstanceAdmin
  ? accessibleInstallations
  : accessibleInstallations.filter(ownsInstallation);

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

isInstanceAdmin is 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 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 (registry.patch-shared-settings, registry.tool-policy) gained 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 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, and AgentProfile.updateMany scoped to {$in: ['pod-1', 'pod-2']}.

  • 28 registry route suites / 103 tests pass
  • backend .ts lint: 0 errors (1206 pre-existing warnings)
  • tsc:check clean

Not run: the full backend unit suite (a 10-line change in one handler plus one test, and CI runs it). Refs TASK-060.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant