Skip to content

fix: allow hosted-native agents into granted pods - #2239

Merged
lilyshen0722 merged 1 commit into
mainfrom
kai/task231-hosted-native-join
Oct 15, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
kai/task231-hosted-native-join

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

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

TASK-231

A hosted agent on the native engine now passes the same in-process predicate at join time that the connector grant already uses at mint and broker call time. A hosted agent on the Durable Object still refuses before installation or membership writes. Plain native admission is unchanged.

The install route chooses the hosted engine once, uses it for the preflight and identity membership check, and writes that same engine to the installation. Client-supplied config.hosted cannot override it. Existing installation membership reads its stored engine. Other preflight callers that carry an existing installation now pass its engine too, keeping the policy consistent across auto-join, pod joins, config changes, and provisioning paths.

Verification

  • Node 22: 12 related backend suites, 108 tests passed at the exact PR head.
  • Backend TypeScript check passed; lint:ts finished with 0 errors (existing warnings). git diff --check passed.
  • New arms: hosted-native install and membership succeed, Durable Object and forged client engine refuse before writes, plain native still succeeds, and an installed hosted-native row is accepted through the undeclared-membership path.

Gate

Head 830d8bff920cbb8c85aae85b574f6c756a39d004, based on main d37a8600. Vera code gate required. Do not merge before required CI and exact-head clearance.

Co-authored-by: kai <WyJrYWkiLCJkZWZhdWx0Il0@agents.commonly.invalid>
Agent-Model: codex/gpt-6-sol/medium

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review at 830d8bf (eng lead). TASK-231: #2220's join policy now counts a hosted agent on the native engine as in-process, the same widening #2224 gave grant eligibility.

  • One predicate: runsInNarrowedProcess decides both grant eligibility and the agent-join check, so they can no longer disagree.

  • No client-chosen engine reaches the join check:

    • install passes engineForNewHostedInstall() (the instance's HOSTED_ENGINE);
    • provision, reprovision, auto-join, the runtime /pods dedup and self-install pass the stored engine of the source install;
    • the config PATCH passes the stored engine with the requested type.

    config.hosted is kernel-only since #2229, so none of these values can come from a request body.

  • Fails closed: an unset or Durable Object engine, or an unknown type, is still refused. A hosted agent that's stopped but on the native engine counts as in-process, which is safe: it can't act, and if re-provisioned it runs narrowed (the engine is sticky).

  • It also closes the false refusal. Before this, a team with a hosted agent couldn't use connectors in a protected pod at all. Vera's call: a false refusal, not a hole.

Checked at this head: 81 suites touching the join policy, eligibility, the hosted engine and grants, 81/81, 1,107/1,107; lint 0 on the changed sources; 0 CodeQL alerts. Every author is Lily.

Non-blocking (Vera's "collapse the spellings"): runsInNarrowedProcess trims and lowercases the type, while the runtime's in-process routing only lowercases. A padded ' native ' would count as eligible without routing in-process. It can't be exploited: the token issuer refuses every spelling, so no outside poller can exist, and #2229 saves types normalized. Still, routing should use the same predicate. That's a small follow-up for whoever does the collapse.

GATE: PASS

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code gate — PASS at 830d8bff920cbb8c85aae85b574f6c756a39d004

TASK-231 is fixed the way the row specified, and the engine boundary is pinned at every site that reads it. I was wrong twice while reviewing this and record both below, because each wrong turn says something about where the coverage actually is.

One definition, where there were three

runsInNarrowedProcess lands in hostedEngine.ts:41-44 and both policies now call it — outsiderGrantPodEligibility.ts:59 and outsiderGrantAgentJoinPolicy.ts:26/:44. That collapses the three spellings of "in-process" I flagged on the row into one, in the module whose header already says "Every site that has to tell the two engines apart asks this module." The right home.

Deleting eligibility's local configObject is safe: readRuntimeConfig handles the top-level Map (hostedRuntimeService.ts:70, config.get('runtime')), and config is { type: Map, of: Schema.Types.Mixed } (AgentRegistry.ts:245), so nested values are plain objects. The new arms exercise this with a real Map rather than a plain object, which is the case that would have broken.

The engine is kernel-selected at all six call sites — checked individually

site engine comes from
install.ts:466 engineForNewHostedInstall() — env only, computed once as newHostedEngine
agentsRuntime.ts:3153, :3315 hostedEngineOf({ config: sourceConfig }), the source installation
agent-config.ts:275 hostedEngineOf({ config: stored }) — the stored row, deliberately not the request's config.runtime
provision.ts:159, reprovision.ts:60 hostedEngineOf(installation), the loaded row

No request value reaches the predicate anywhere. install.ts computes newHostedEngine once and feeds the same value to the refusal (:467), the stored config (:520) and the membership write (:655), so those three cannot disagree — which is the PR's headline claim and it holds as written.

Two existing layers make the engine unforgeable rather than merely ignored, and I checked both because "we read the stored value" is only conservative unless the stored value is protected: install.ts:519 deletes a client-supplied hosted outright, and KERNEL_OWNED_CONFIG_KEYS = ['hosted'] (configWriteGuard.ts:27) is refused at :90 with kernel_owned_field — above the if (isInstanceAdmin) return null at :99, so not even an instance admin can PATCH the engine.

Mutations — three, each killed, each for its reason

  • Install route reads the request's engine instead of newHostedEngine for the refusal while still storing the kernel's: 2 failed, exactly the two install arms. This is the bypass the design exists to stop, and it is pinned.
  • Drop the hostedEngineOf === NATIVE conjunct from runsInNarrowedProcess so any hosted install counts as in-process: 5 suites, 7 arms red — eligibility (…Durable Object (no engine recorded) is ineligible, …engine other than native is ineligible), the join policy both forms, ensureAgentInPod, auto-join's Map copy, and the install route. The boundary is observed from every direction.
  • Revert the join policy to the bare literal (the pre-PR behaviour this row is about): 4 arms red across 4 files.

Restored after each; tree back to 393ad475, clean git status.

Runs

  • Node 22, 102 suites across the touched surface: 914 passed, 0 failed. tsc --noEmit -p tsconfig.typescheck.json: 0 errors.
  • git merge-tree --write-tree origin/main HEAD: clean.
  • CI per-SHA, every check complete, every one success or skipped: Test & Coverage, Service Tests (Tier 1 — real DBs), E2E Tests, CodeQL ×3, PR targets main, Stale-base merge guard, Detect secrets, Chart Lint, ADR numbers, Rule numbers, Source changed ⇒ version bumped. That last one also has one cancelled instance from an earlier push, superseded by a success of the same name. I held this verdict until the run finished rather than file a conditional clearance.

Two things I got wrong in this review

I grepped the test tree for assertAgentRuntimeMayJoinPod and concluded no test asserted the install route's pass-through. Then I mutated it anyway and two arms went red: registry.hosted-cap-gate.test.js:213 and :229 exercise the route and assert on outcomes, so they never name the function. I had a HOLD drafted off that grep and the mutation is the only reason it is not in this verdict.

Then I thought "client-forged native refuses" was an untested claim. Reading those two arms in full shows it is tested in both directions: :213 sets HOSTED_ENGINE=native with the request asking for durable-object and asserts the install stores native and ensureAgentInPod is called with ('hosted', 'native'); :229 sets HOSTED_ENGINE=durable-object with the request asking for native and asserts 403 plus no install and no membership write.

One distinction worth keeping straight, since the claim spans two PRs: this PR closes a forged engine. A forged runtimeType: 'native' still short-circuits assertAgentRuntimeMayJoinPod — what makes that harmless is #2224's token issuer refusing any-spelling native with in_process_agent_no_token (tokens.ts:202-217), so such a row cannot be driven from outside the backend. This PR relies on that; it does not re-establish it. Worth knowing if anyone ever relaxes the issuer.

Scheduling, carried from the row

Measured read-only against production before this PR existed: 8 active hosted installs, 0 on engine: native, 0 live room grants. So the bug was latent. #2236 migrates the existing hosted installs to native, which is what would have made it live — so this landing first is the right order, and @connector-ops has that.

Clearance is for 830d8bff on main at 1ad396dc. A code-touching push spends it.

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

GATE: PASS

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