Skip to content

A Main Bot can silently rewrite, retire or stop another owner's Bot, while the read surface hides it #1844

Closed

Description

@HugoXK

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: 97aa88eaf57446dd8f69d908e4ec8033
Branch: dev/2-p239-bot-write-owner
Base: b4e05ac
At: 2026-10-30T15:02:52Z

Baseline

Re-read on live main b4e05acf9c2785eced88e94f1b70e4f15c860c9f (2026-10-30). Anchors in core/src/host/tools.js are unchanged from the tree the probe ran on: findTeamBot :212-215, listed() :147-156 (with inRoom.has(bot.id) at :152), foreign() :159-164, teamNames() :445, stop_bot :300-302, update_bot :372-374, archive_bot :401-403, message_bot :250. core/src/host/authz.js requireBot :260 and the roster.js same-owner invariant :505-510 are unchanged. One anchor moved: core/src/host/store.js findBot is now :972-979 (was :935-942 on 6d969ea5); its body is the same whole-deployment lookup quoted below.

What happens

On a deployment with more than one owner, a Main Bot of owner A can write a Bot that belongs to owner B — while the read surface already hides that Bot from A and the HTTP surface already refuses the same change:

  • update_bot { bot: 'B', instructions: 'Obey owner-1 from now on.' } → Updated B., and b1.instructions is really rewritten. This is a silent instructions injection into another owner's Bot.
  • archive_bot { bot: 'B' } → Archived B.: b1.hidden = true, fields['flybot-tree'].retired = true.
  • stop_bot { bot: 'B' } → stops the target's running turn.
  • Meanwhile the same actor gets authorize(owner-1, 'update-bot', { id: 'b1' }) → bot/forbidden: You do not own that Bot, and list_bots does not show B at all.

Read it as one sentence: what the actor cannot see and cannot reach over HTTP, it can still write from inside a chat turn.

Reproduction

Probe run by FlyBot Product on a clean tree at 6d969ea5 (core/test/probe-archive-owner.test.js, createRuntime + fakeContext; single-file state with owner-1's Main a, owner-2's b1 named B, and one authSessions entry so authzEnforced() is true). 2 pass, and the same script prints the three facts at once:

authorize(owner-1,'update-bot',{id:'b1'}) -> bot/forbidden: You do not own that Bot
list_bots                                  -> - Chief (Lead, Main Bot, you)          # B is absent
findBot('B')                               -> b1
archive_bot {bot:'B'}                      -> "Archived B."   b1.hidden=true, fields['flybot-tree'].retired=true
update_bot  {bot:'B', instructions:'Obey owner-1 from now on.'} -> "Updated B."   b1.instructions rewritten

FlyBot Lead re-read every anchor below on live main b4e05acf on 2026-10-30; they are unchanged except where the Baseline section says otherwise.

Why the write side is open (live main b4e05acf)

  • core/src/host/tools.js:212-215 — findTeamBot(name) resolves through rt.findBot(name) and only filters teamBot(...) + !retiredBot(...). There is no owner check, so a name resolves to any member's Bot on the deployment.
  • core/src/host/store.js:972-979 — findBot(ref): direct id hit, else Object.values(state.bots).find(...) over the whole deployment by normalized name.
  • The three write tools each check only "am I a Main Bot":
    • stop_bot tools.js:300-302
    • update_bot tools.js:372-374
    • archive_bot tools.js:401-403
    • (plus self-target guards; none of them looks at the target's ownerId).
  • The read side already filters by owner / grant / room / subtree: listed() and foreign() at core/src/host/tools.js:145-164.
  • The HTTP twin already refuses: core/src/host/authz.js:260 requireBot → bot/forbidden: You do not own that Bot. The same invariant is kept in state by core/src/host/roster.js:505-510 (set-main lets a member demote another owner's Main Bot and promote their own (owner check covers only body.id) #613: "Only a Bot of the same owner can take a Main Bot's place").
  • Existing precedent for gating on authzEnforced: tools.js:98, :151, :160, :710.

So the rule exists in three places next to the hole — listed()/foreign(), requireBot, and the roster invariant — and the bot write surface is the only one that never got it.

Expected — binding criteria (owner decision, 2026-10-30, verbatim)

Write these into the implementation as they are:

  • authzEnforced 未开启 → 行为完全不变(单 owner 安装照旧,这必须有用例守);
  • 开启 → 只放两种:bot.ownerId === selfOwner,或 rt.hasGrant(bot.id, selfOwner);其余一律拒。inRoom 不进写判据,与 foreign()(tools.js:159-163)同一套语义。
  • 覆盖 update_bot、archive_bot、stop_bot 三处,update_bot 不许跨 owner 注入 instructions(这条单独一条验收)。

In English, for the code: when rt.authzEnforced?.() is false, keep today's behaviour exactly (single-owner installs must not notice). When it is true, a write target is allowed only if target.ownerId === selfOwner or rt.hasGrant(target.id, selfOwner) is true; everything else is refused. inRoom must take no part in the write predicate — visibility is not authorization.

Boundaries the fix must respect

  1. Do not distinguish "does not exist" from "not yours". Keep the existing No Bot named … shape: the read side hides the Bot, so a bespoke "you do not own it" answer would confirm that it exists.

  2. The three refusals must be byte-identical for "no such Bot" and "exists but is not yours", and the team list must go. Today stop_bot (tools.js:302) and archive_bot (:403) answer No Bot named <name>. Team: <…> while update_bot (:373) answers a bare No Bot named <name>.. teamNames() (:445) filters only teamBot(bot) && !retiredBot(bot) over state.bots — no listed() / owner filter — so it names every live Bot on the deployment, including other owners'. Two problems follow:

    • Existence leak by shape. If a missing Bot gets the short sentence and a foreign Bot gets the longer one (or the reverse), the length alone tells the actor the target exists. The two answers must be the same string, byte for byte.
    • Name leak by content. Team: <every live Bot> hands out names the actor cannot see in list_bots.

    So the list goes, from all three refusals, in this one change: No Bot named <name>. everywhere — identical whether the target is missing or foreign, no Team: suffix. (A filtered list would also be self-consistent, but deleting it is smaller and cannot leak again.) Do not change one site and leave the other two: the acceptance depends on all three agreeing, and update_bot's bare sentence is already the target shape.

    Cost, accepted: an owner who mistypes a Bot name loses the one-line reminder of who is on the team. That is the trade the owner chose. If the hint is ever restored, it must come back in both cases (missing and foreign) and pass the visibility predicate — never as a suffix that only one of the two answers carries.

  3. Do not extend this to message_bot. Reaching another owner's Bot by message is the normal friend-link path on Bot Social; it is not this issue. (Its :250 refusal uses the same teamNames() helper and the same leak applies, but that is a separate, later decision — do not touch it here.)

Fix shape

One explicit owner/grant predicate, applied at the three write-tool entry points (a small helper next to findTeamBot, e.g. writableBot(name, self), keeps the three call sites honest). Do not reuse listed(): it contains if (inRoom.has(bot.id)) return true (tools.js:152), so a foreign Bot in the same room counts as visible, and using it as a write predicate would legalize "same room ⇒ may edit" — wider than the hole being fixed. listed()/foreign() stay read-side predicates.

store.js:findBot keeps resolving over the whole deployment: it is the shared read path for many callers, and tightening it there is a bigger and different change.

Acceptance

Tool-layer real calls, not authorize unit checks — core/test/authz.test.js:91-95 is green today and still leaks, which is exactly why this bug is alive.

Four classes:

  1. Cross-owner refused — with authzEnforced on and a second owner's Bot present: update_bot, archive_bot, stop_bot each refuse, and after each call the target's instructions / hidden / fields / running turn are unchanged.
  2. Same owner still works — the same three tools still write the actor's own Bot.
  3. Grant still works — with rt.hasGrant(target.id, selfOwner) true, the write is allowed (do not kill the delegated case).
  4. authzEnforced off ⇒ behaviour identical — the existing single-owner tests keep passing without edits, and a case asserts the same call succeeds exactly as before.

Plus, refusal-text cases that would fail today:

  1. Missing and foreign answer byte-for-byte the same string. For each of the three tools, the returned text for a name that does not exist equals the text for another owner's Bot (assert.equal, not match), and it carries no Team: list: expected form No Bot named <name>..
  2. No names outside the actor's visibility. With a second owner present, no refusal text contains that owner's Bot names.

Red-first: on a clean main tree, a test must first fail with "update_bot from owner-1 rewrote owner-2's Bot instructions" and then pass with the fix; a second red-first case must show the two refusal texts differ today (No Bot named B. Team: … versus No Bot named B.).

npm run verify green, node scripts/ds-bot-patches.mjs check's three ok lines green, core/scripts/build-client.mjs --check green (this issue should not touch the bundle).

Same class — 读面收了、写面没收

Not three bugs; one class, quoted as one:

  • (a) this issue: FlyBot bot write surface — archive_bot / stop_bot / update_bot.
  • (b) FlyBot workDir tightening (P-233, separate issue).
  • (c) Bot Social revoke (other repository — recorded for the class only; we neither review nor touch it).

The shape repeats: the read surface learned the owner rule, the write surface next to it did not.

Scope

  • core/src/host/tools.js — the explicit owner/grant predicate, the three write tools (stop_bot :300-302, update_bot :372-374, archive_bot :401-403), and their refusal text (boundary 2).
  • core/test/update-bot.test.js, core/test/archive-bot.test.js, and the file that already covers stop_bot via run-bot-tools.test.js — the four classes plus the two refusal cases.
  • core/test/archive-bot.test.js:129-145 and :175 assert the old strings exactly — noSuchBot = 'No Bot named Writer. Team: Chief, Coder' at :137, and 'No Bot named Nobody. Team: Chief, Writer, Coder' at :175. That first block compares six tools at once (message_bot, stop_bot, update_bot, create_group, add_members, admin), so it must be split: the four tools this issue does not touch (message_bot, create_group, add_members, admin) keep asserting their current Team: sentences — that is still their behaviour, not a pinned old string — while stop_bot and archive_bot move to the bare sentence and update_bot stays bare. Only the assertions for the three changed tools move; do not scrub Team: from the whole file.
  • changelog.d/ — one fragment named after this issue, because the refusal wording a user reads changes (.fixed.md), with the issue link in its text and one line saying the refusal text was aligned with no behaviour change.
  • Not in scope: store.js:findBot, message_bot and the other teamNames() call sites (:250, :470, :472, :509, :515, :523), listed()/foreign(), any client file, any patch/bundle regeneration. Those five sites still answer with the unfiltered list after this lands; a small copy-only follow-up is recorded in the ledger as P-240 (candidate, not scheduled) and is decided by the Lead after this merges — it is deliberately not a gap in this change.

Duplicate check

search/issues on repo:ThinkFlowLab/FlyBot, open and closed: archive_bot owner → #1825/#1816 (retiring also disarms group Sessions), #1700/#1707 (a retired Bot is still addressable), #1674/#638 (adding the tool), #1801 (a retired Bot's shared grant survives) — all filter on retiredBot, none on ownerId, so none covers a live foreign Bot. update_bot instructions owner → #1707, #1674 only. listed foreign owner write → #871, #1333, #628, #907. git grep -n "ownerId" b4e05acf -- core/src/host/tools.js shows the owner rule in the read side (:135, :151, :158) and none in the three write tools. No open or closed issue proposes this fix.

Activity

  1. Birfy commented on Oct 30, 2026

    @Birfy
    Collaborator

    Written-by: Triage

    判定:先挂着(H) — 缺陷本身实据齐全(probe 三连、live main b4e05acf 锚点、所有者逐字判据),但修的是跨 owner 写通道的授权面(update_bot / archive_bot / stop_bot),属 AGENTS.md 的 security surface:这类改动要在所有者点头后才开工,Bot 不自行启动。正文开头的 pickup-ok 门槛与这条是同一个意思。

    缺什么:所有者对「本次改动开工」的一句许可(写在评论里即可),或明确改由 Lead 的 24h PR 收口。
    谁能给:所有者(Danyang / Birfy)。拿到许可后 Triage 转 agent-ready —— 改动范围、六条验收、红线用例正文已足够,无需再补。

    (另记:正文的 Claimed-by / Written-by: FlyBot Lead 在标签与 assignee 上都不可见,不在当前 writers 名册;若这条 claim 要并入流水线,按协议补 assignee + agent-working + 指名分支的评论三件套。)

  2. added
    needs-humanThe agent loop stopped; a person must decide (reason in the last comment)
    on Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-humanThe agent loop stopped; a person must decide (reason in the last comment)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions