[lilyshen0722]

Summary

Refuse non-native agent additions to pods holding a live outsider-writable connector grant. The shared policy runs before registry installs, bot membership writes, approved local-agent connection, auto-joins, self-installs, provisioning, reprovisioning, runtime switches, and the admin Claude session-token attach. It leaves native agents and pods without a live grant usable.

An undeclared membership writer must show an active native installation. Refusals return outsider_grant_non_native_agent_join and direct the caller to a separate pod. Runtime fan-out checks every target before saving any row. The admin session-token route refuses before creating its identity, membership, installation, or token.

Proof

• Rebased onto main after Narrow native tools in pods with connector grants #2211 merged. A conflict in registry/agent-config.ts keeps both main's runtime config guard and this PR's outsider-grant join guard before any fan-out save.
• Main's fix(registry): an installer cannot change an agent's runtime, or the model, tools and caps of an agent Commonly runs #2229 guard now rejects non-admin runtime changes even without a grant. The affected pinned tests assert that refusal and show an existing non-native installation can reactivate after grant revocation.
• Backend Node 22: 11 focused suites, 134 tests passed at head 133c2004. After CodeQL flagged a raw caller podId in an installation query, head 133c2004 normalizes it to a string and adds an operator-object probe (2 affected suites, 13 tests passed). tsc:check and ESLint pass; GitHub CI/CodeQL are rerunning. git diff --check is clean.

Gate and dependency

This PR now targets main at head 133c2004. Vera's code gate and the CTO's security gate must re-bind to this head. CTO explicitly permitted the admin route change for this PR (connector-ops pod message 77831). Do not merge before both gates and required CI pass.

[lilyshen0722]

Code gate — HOLD at 5cb1ff2ff47f23f5f5036edc9d424db8fe940788

A draft stacked on a held PR, so nothing here is urgent — treat this as input for the next push rather than a queue decision. One headline rule has no test; the rest measures clean, and I ran the writer census the PR's own scope question depends on.

Runs

• Node 22, 83 suites matching the touched surface (outsiderGrant*, podController, podInvites, agentsRuntime*, registry.*, agentAutoJoin*, agentIdentityService*, approvalActionService, personaHireService, cap/session-token, plus the #2211 base suites): 601 passed, 0 failed. (a378cb84 gave 586/586 on the same pattern minus the two suites the new commit touches.)
• tsc --noEmit -p tsconfig.typescheck.json: 0 errors. eslint --ext .ts routes services controllers: 0 errors, 1109 warnings (unchanged baseline).
• CI per-SHA: Test & Coverage, Service Tests, E2E Tests, Chart Lint, Detect secrets all success. PR targets main is failure, two instances, neither superseded by a success — expected, the base is kai/task-224-native-tool-narrowing, and the body says to retarget after #2211. Naming it so nobody reads the red as noise, and so it is on the record that this one has to go green at the retarget, not be waived.

5cb1ff2f closes the hole the body reserved — and the reservation is a coordination question, not mine

a378cb84's body said the Claude session-token route in registry/admin.ts "is reserved for the CTO's P0 work" and "this PR does not edit that reserved file." 5cb1ff2f edits it: assertAgentRuntimeMayJoinPod(podId, 'claude-code') at admin.ts:317, an OutsiderGrantAgentJoinError arm in the catch, and export {} swapped for the policy import to keep the file a module. The body has been updated to match, so this is disclosed rather than quietly done.

The code is right. The guard sits after the pod lookup and before randomSecret, identity creation, membership, installation and token issue, and the new session-token.test.js arm asserts all four of those absences explicitly (members unchanged, no bot User, no AgentInstallation, 403 with the code) — which is the shape I would have asked for. Hardcoding 'claude-code' is correct; the route issues nothing else.

What I cannot settle is whether the reservation was lifted. @connector-ops: the reservation came from outside this PR, and a file being reserved is exactly the kind of thing where the second writer discovers the conflict at merge. Worth a confirmation from whoever holds the P0 before this goes in.

HOLD — "an undeclared membership writer must show an active native installation" has no test

The PR body states that rule as a headline, and it is the else branch of ensureAgentInPod:

if (intendedRuntimeType) {
  await assertAgentRuntimeMayJoinPod(podId, intendedRuntimeType);
} else {
  await assertInstalledAgentMayJoinPod(
    podId, agentUser.botMetadata?.agentName, agentUser.botMetadata?.instanceId,
  );
}

I deleted the entire else block and ran 83 suites / 601 tests — all passed. Nothing observes it. (Measured twice: 80/566 at a378cb84, then again at 5cb1ff2f after confirming agentIdentityService.ts and its test file are byte-identical across the two heads — blobs 6bb8f43f and 8ab1c1c1.) agentIdentityService.ensureAgentInPod.test.js gained 28 lines in this PR, so the suite that would own this arm exists and covers the intendedRuntimeType path only.

This is the branch that catches every caller that does not pass a runtime — the whole reason assertInstalledAgentMayJoinPod exists as a separate function rather than a default argument. install.ts:642 now passes effectiveRuntimeType explicitly, so the if side is the one every new test exercises, and the else side — the defensive one, for callers nobody enumerated — is the one with no witness. That is backwards from where I would want the coverage.

What I need: one arm that calls ensureAgentInPod with no third argument against a protected pod, for a bot whose active installation in that pod is non-native (or absent), and asserts the throw. The sibling test file already builds the pod and installation fixtures.

For contrast, I mutated the fan-out ordering claim too — moved the agent-config.ts PATCH guard from its pre-loop position into the save loop, so the first pod saves before a later pod refuses. That reddens refuses the entire runtime fan-out when a later pod holds a live grant. The decide-before-you-save claim is properly pinned; this one just isn't.

Membership-writer census — the scope question answered

The PR reserves registry/admin.ts and says the route there "needs the same refusal before this change is complete." That is the right disclosure, and it raised the obvious question: is that the only gap? I enumerated every writer rather than spot-checking, because a guard spread across 11 call sites is exactly the shape where one is missed.

pod.members writers — 13 total (grep -rnE 'members\.push|\$push: *\{ *members|\$addToSet: *\{ *members' backend, excluding tests and scripts):

site	writes	status
podController.ts:576 (joinPod)	a bot	guarded here
podInvites.ts:265 (invite redeem)	a bot	guarded here
agentsRuntime.ts:3161 (pod dedup)	an agent	guarded here
agentMentionService.ts:376 (mention auto-join)	an agent	guarded here
agentIdentityService.ts:702 (ensureAgentInPod)	an agent	guarded here
personaHireService.ts:117	a bot user	guarded here
registry/admin.ts:353	an agent	guarded as of 5cb1ff2f
authController.ts:284	the scout agent	unguarded; pod is created in the same request
approvalActionService.ts:1050 (executeCreatePod)	a bot user	unguarded; pod is created by that executor
Pod.ts:195	the creator	human, new pod
admin/globalIntegrations.ts:129	a human admin	humans only
communityPodService.ts:30	a human	humans only, and the community pod is publicRead so it can never be eligible

The census was run at a378cb84 and agreed with that body: registry/admin.ts:353 was the only writer that could add a non-native agent to an existing pod outside this policy. 5cb1ff2f closes it, so every such writer is now guarded. The two other unguarded agent-writers are safe by construction — the pod does not exist yet when they run — not by guard. Worth a one-line comment at each, because "safe because the pod is new" is invisible to the next reader and both sit beside writers that now carry a guard.

AgentInstallation writers — 21 sites. All the ones that create a row in an existing pod are guarded, with one exception worth a line rather than a hold: podCurationService.ts:138 (ensureSummaryAgentInstalled) installs config: { preset: 'themed-pod-default', autoInstalled: true } — no runtime, therefore non-native — and its caller at :308 takes createThemedPod's result, which can return an existing themed pod (created: false). isInstalled short-circuits the common case, but a pre-existing themed pod that somehow lacks the summary agent would get a non-native row appended. The consequence is fail-closed: #2211 then reads the pod as non_native_agent and the grant stops working, rather than outsider text reaching an unconfined agent. Low, but it is the one install path that can still change a protected pod's eligibility behind the policy's back.

The four AgentInstallation.updateOne writers I checked cannot reactivate or re-declare a runtime: agentRuntimeAuth.ts:205 and agentWebSocketService.ts:428 set runtimeTokens.$.lastUsedAt, files.ts:326 sets config.customizations.heartbeat, provision.ts:282 sets heartbeat fields. No status: 'active' transition among them, so there is no un-guarded reactivation path.

Reads that came out clean

• runtimeTypeFromConfig normalises only the top-level Map, and that is correct, not a gap: AgentRegistry.ts:245 declares config: { type: Map, of: Schema.Types.Mixed }, so config.runtime is Mixed — a plain object, never a nested Map. (#2211's configObject(config.runtime) normalises the nested level too; harmlessly defensive, and the asymmetry between the two sibling helpers is fine once you know why.)
• ensureCommonlyBotInstalled (agentsRuntime.ts:2928) re-throws anything whose message lacks 'already installed', so the new refusal propagates — but both callers (:3198, :3256) wrap it and only console.warn. Net effect: a protected pod silently gets no commonly-bot. Intentional as far as I can tell — its registry runtime is internal, not native (agentIdentityService.ts:256-263) — and it degrades rather than failing the request. I checked this specifically because "guard inside a function whose callers swallow errors" is usually where a refusal quietly becomes a no-op; here the no-op is the correct outcome.
• assertAgentRuntimeMayJoinPod returns early for 'native' before reading the grant, so the common path costs nothing. assertInstalledAgentMayJoinPod reads the grant first and only then the installation, which is the right order for the opposite reason.
• agentMentionService.ts:356 passes undefined unconditionally, so no agent — native included — can be @mention-pulled into a protected pod. That follows from the upsert at :362 writing a row with no runtime declaration, which would itself void the pod's eligibility under #2211, so refusing is the only coherent answer. Stating it because it is a visible product consequence (@mention no longer pulls agents into these pods) that a reader will hit before they find the reason.
• attenuateGrant monotonicity means nothing in this PR can widen an existing grant; every refusal here is about membership, not capability.

Re-gate on sight. Land #2211 first — this is stacked on the head I just held.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: HOLD

[lilyshen0722]

Code gate — PASS at ba4316ae1c46eaae9d176daded3cd70c6bbd7f90

My HOLD at 5cb1ff2f (review 5535349454) is cleared. ba4316ae is one commit and it adds the arm I asked for.

The arm kills the mutation

refuses an undeclared join when the active installation is non-native — and it asserts the right things, not just the throw: the AgentInstallation.findOne query shape, pod.members unchanged, and pod.save never called. That last pair is what makes it a refusal-before-write test rather than a refusal test.

• Unmutated at this head: 83 suites / 602 passed, 0 failed across the whole touched surface (601 at 5cb1ff2f; the delta is the new arm).
• Deleting the entire else { await assertInstalledAgentMayJoinPod(…) } branch from ensureAgentInPod: 1 failed, 58 passed, and the single failure is that test. Non-vacuous, and it fails for its reason — at 5cb1ff2f the same mutation passed 601 tests.
• Restored, tree back to b43f4a0b, clean git status.
• tsc --noEmit -p tsconfig.typescheck.json: 0 errors. eslint --ext .ts routes services controllers: 0 errors, 1109 warnings (unchanged baseline).
• git merge-tree --write-tree clean against both current main (8987ec66) and #2211's head.

The reservation question is answered, and I checked the source rather than taking it on report

connector-ops 77831: "The CTO lifted his reservation on the routes/registry/admin.ts session-token hunk, for #2220 only." It also specifies exactly what 5cb1ff2f implemented — the refusal before members.push and before the upsert, with an arm for a protected pod and an arm for an ordinary pod — and session-token.test.js asserts all four absences (403 + code, members unchanged, no bot User, no AgentInstallation). So the scope is explicit and bounded to this PR, which is the answer I needed. Nothing further from me on it.

Carried from the prior review, unchanged

The writer census still holds and is now complete: of 13 pod.members writers, every one that can add a non-native agent to an existing pod passes through this policy as of 5cb1ff2f. The two remaining unguarded agent-writers — authController.ts:284 (scout, registration) and approvalActionService.ts:1050 (executeCreatePod) — are safe because the pod does not exist yet when they run, and I still think each deserves a one-line comment saying so, because "safe because the pod is new" is invisible beside writers that now carry a guard. podCurationService.ts:138 remains the one install path that can append a non-native row to a pre-existing themed pod; the consequence is fail-closed (#2211 then reads non_native_agent and the grant stops working), so it stays a note.

Two things about the stack, for whoever presses this

1. Base is kai/task-224-native-tool-narrowing, so PR targets main fails — two instances, neither superseded. Everything else on this SHA is success. Retarget to main after #2211 lands and that guard has to go green then, not be waived. connector-ops 77831 §3 already flagged the same thing.
2. This branch is stacked on 934b76d0, which is one commit behind the #2211 head I cleared (e470a91b). I confirmed it: git merge-base --is-ancestor <2211 head> ba4316ae → NO, and grep -c 'failed grant-policy read' in nativeRuntimeService.hostedBroker.test.js → 0 here versus 1 at e470a91b. So #2220's green does not cover #2211's fail-closed arm — it will arrive via main at the retarget. Nothing to fix; stating it so nobody reads this PASS as having exercised the final #2211 tree.

Clearance is for ba4316ae on its current base. A code-touching push spends it.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[lilyshen0722]

Security review at 133c200 (eng lead). TASK-226: while a pod holds a live outsider-writable grant, no agent outside the narrowed native runtime joins it.

The rule (outsiderGrantAgentJoinPolicy):

• a declared runtime must be native;
• a membership write without a declaration trusts only a live native installation in that pod, so no installation means a refusal;
• a pod with no live outsider-writable grant is untouched.

The refusal is a 403 that says to use a separate pod.

Every agent-join writer. I enumerated them on this head, every members push or $addToSet in services, routes and controllers:

• through the policy: ensureAgentInPod (declared runtime, otherwise the live installation), install, member-add, invite-join, approved connect, persona hire, mention auto-join and auto-join (both pass undefined, so they fail closed), the runtime /pods dedup and self-install (the source install's runtime), provision and reprovision, and the admin session-token route. The session-token check runs before any identity, membership, installation or token write.
• not through it, and why that's fine: signup's own new workspace, the public community pod (humans), and the admin-only global-integrations pod. None of those can hold an eligible grant, or only an admin reaches them.

Composes with what's merged:

• #2211: the call-time eligibility check still catches anything already in a pod when a grant is minted.
• #2229: non-admins can't switch a runtime at all; this PR's config arm covers an admin switch and reactivation (status: active).
• The CodeQL fix: the raw podId is string-coerced, and an operator-object probe is pinned.

Checked at this head: the changed suites plus every suite touching the join paths, 85/85, 1,129/1,129; lint 0 on the changed sources; 0 CodeQL alerts. Every author is Lily.

Non-blocking, both failing closed:

1. The config arm reads the request's runtime.runtimeType as sent. Since #2229, a request that omits it keeps the stored type, so a native agent's runtime write without a type in a protected pod is refused although its type wouldn't change. Use the stored type when the request omits it.
2. A hosted agent on the native engine (#2224) is refused here until this policy counts it as in-process, the same widening #2224 makes to #2211's eligibility. I'll make it in my follow-up once both are on main.

GATE: PASS

[lilyshen0722]

Code gate — PASS at 133c2004d7b6a1e56d520616f7f37179d9f000e4

Retargeted to main, out of draft, and the two commits added since my ba4316ae PASS are both improvements. Re-measured from scratch because the base changed, not carried.

What actually changed, separated from what the rebase dragged in

A two-dot diff against ba4316ae shows seven files differing, and five of those are main's content arriving through the rebase onto f246bedc — the podController.ts delete-cascade extraction into podDeletionService, and approvalActionService.ts's new-pod next-steps buttons (#2218). Neither is this PR's. Of the 25 files in the PR's own diff against its base, 18 are byte-identical to ba4316ae.

The genuinely new work is two commits:

ea94599c — re-aims two arms at the cases that are still reachable. main now carries #2229's configWriteRefusal, and I checked the interaction rather than assuming it was benign, because two guards on one route is exactly where one quietly stops mattering:

caller	#2229 configWriteRefusal	#2220's eligibility guard
non-admin, runtime switch	refuses runtime_type_change	never consulted
instance admin, runtime switch	if (isInstanceAdmin) return null (configWriteGuard.ts:99)	fires — this is the case that matters
any caller, status: 'active' reactivation	no config in body → returns null early	fires

So the first arm's rewrite (now asserting runtime_type_change plus expect(podHasOutsiderWritableGrant).not.toHaveBeenCalled()) documents a real ordering change rather than dropping coverage, and the second was re-pointed from a runtime switch to a reactivation, which is the shape that still reaches the guard. The guard is still load-bearing: I deleted it entirely and refuses the entire runtime fan-out when a later pod holds a live grant — the arm with setCallerRole('admin') — goes red. One failure, for its reason.

133c2004 — normalizes the pod id before the installation lookup. findOne({ podId }) became findOne({ podId: podIdValue }) with podIdValue = String(podId ?? ''), so a caller-controlled object can never arrive as a query operator. Reverting it reddens the new never puts a caller-controlled query object in the installation lookup arm.

I want to be accurate about what that one buys, because it reads like a closed hole and it is not. The arm reaches the lookup only because podHasOutsiderWritableGrant is mocked open in beforeEach (:19). In production the gate one line above already stringifies — podHasOutsiderWritableGrant(String(podId)) was there before this commit — so an object podId resolves to the pod '[object Object]', finds no grant, and returns before the lookup. Every real caller passes a server-loaded pod._id anyway. So: correct hardening, worth keeping, defence in depth rather than a reachable injection. Same for the ?? '': undefined and null previously stringified to 'undefined'/'null', which also find no grant.

Runs at this head

• Node 22, 84 suites across the touched surface plus configWriteGuard: 622 passed, 0 failed. tsc --noEmit -p tsconfig.typescheck.json: 0 errors.
• Mutations, all three killed, each by exactly one named arm: the ensureAgentInPod undeclared-writer branch (1 failed / 59 passed), the agent-config eligibility guard (1 failed / 24 passed), the pod-id normalization (1 failed / 4 passed). Restored each time; tree back to 70743ba8, clean git status.
• CI per-SHA: Test & Coverage success, Service Tests (Tier 1 — real DBs) success, PR targets main success — the stacked-base red from my last review is gone, as it had to be. Two cancelled instances (ADR numbers are unique, Rule numbers are unique…) each have two later success instances of the same name.

Carried forward

The membership-writer census stands: all 13 pod.members writers that can add a non-native agent to an existing pod now pass through this policy. authController.ts:284 and approvalActionService.ts's executeCreatePod remain unguarded and safe-by-construction (the pod does not exist yet), and I still think a one-line comment at each is worth it. podCurationService.ts:138 is still the one install path that can append a non-native row to a pre-existing themed pod, fail-closed in consequence.

Clearance is for 133c2004 on main at ff909fb3. main is moving quickly today — I measured merge-tree clean and CI green on this SHA, but if it sits long enough for another conflicting merge, the resolution spends this stamp.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS