From f4f7e0d04787a10c55a52cdc89a6d83ed09fa980 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:46:10 +0800 Subject: [PATCH] social: a read-only window's read endpoints write no settings file (#2231) `social/remote-bots` and `social/friends` are declared reads, and in a read-only window they still reached `saveSettings`: the one-shot inbound backfill, the hidden-remote aging and the accept-notice cleanup rewrote `flybot-social-settings.json`, so a window the host had just told to close the other one kept editing the file the writable window owns, and a second window could flush its older copy back over what the first had saved. Guard the save, not the derivation: `saveSettings` returns without writing while `flybot.readOnly?.()` is truthy, the signal the seven other `flybot.readOnly?.()` sites in this file already use. The derived values are still computed and returned for that window, and a writable window persists exactly as before. --- changelog.d/2231-settings-readonly.fixed.md | 1 + docs/subsystems/social.md | 2 +- plugins/social/src/social.js | 9 ++- plugins/social/test/social.test.js | 88 ++++++++++++++++++++- 4 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 changelog.d/2231-settings-readonly.fixed.md diff --git a/changelog.d/2231-settings-readonly.fixed.md b/changelog.d/2231-settings-readonly.fixed.md new file mode 100644 index 000000000..9427422a9 --- /dev/null +++ b/changelog.d/2231-settings-readonly.fixed.md @@ -0,0 +1 @@ +A read-only FlyBot window no longer rewrites `flybot-social-settings.json` from its read endpoints: `social/remote-bots` and `social/friends` still derive the inbound backfill, the hidden-remote aging and the accept-notice cleanup in memory, but they write nothing, so opening a second window cannot create that file, age a hidden friend on it, or put an older copy back over what the writable window saved ([#2231](https://github.com/ThinkFlowLab/FlyBot/issues/2231)). diff --git a/docs/subsystems/social.md b/docs/subsystems/social.md index 671b95929..a36360ab4 100644 --- a/docs/subsystems/social.md +++ b/docs/subsystems/social.md @@ -152,7 +152,7 @@ Creating a group opens its Session with zero turns, and the host draws no sessio ### The server in Settings (#337) - **Where it comes from.** `config.server` in the profile patch is the default. A server picked in Settings → **Bot Social** is kept in `flybot-social-server.json` (mode 600, next to the credentials file) and wins while it exists; choosing the default again removes the file. It is written through `flybot-social-server.json..tmp`, and that temp name is asked as a file first (temp-name sweep): a symbolic or hard link left there refuses the save instead of putting the choice outside the folder and renaming the link into place — `lstat` reports a hard link as a plain file while its bytes belong to another name too, so the guard also requires `nlink === 1`, and the plain leftover of a crashed host stays replaceable. Nothing configured anywhere behaves as before (the start fails with `server URL is required`, `Social is not running: …`; the page says **No server set**). The page reads `social/server` (a read; `{ server, source: settings|config|none, defaultServer, state: connected|connecting|not-configured|error, handle, error, canChange, known[], platform? }`: never a token) and calls `social/server-check` (it makes the host fetch an address the caller names, so it follows the write rules like `social/server-set`). -- **What may be written where.** `flybot-social.json`, the `.bak` copy `startSocial` keeps for another server (`flybot-social.json.`) and `flybot-social-settings.json` are written by one function, and it asks the leaf with `lstat` before the first byte: a missing file is created (mode 600), a plain file of its own (`nlink === 1`) is rewritten, and a symbolic link, a hard link, a directory or any other kind refuses the write — nothing is written there, the file behind a link keeps its bytes, one line names the path and carries no value from the file, and `startSocial` fails with that refusal instead of running on a link. The refusal is asked **before** onboarding: the onboarding is a real home on the server and a round-trip, so a start whose credentials path cannot be written refuses first and never spends one on a token it would drop. A refused `.bak` leaves the credentials in use untouched, and its name is asked in that same pre-check: the backup name follows from the server in the credentials file already on disk (`flybot-social.json.`), a local read, so a link left there refuses the start before the onboarding rather than after it, and a free name still keeps the old credentials under their server. `flybot-social-settings.json` is a different shape and stays as it is: it is read (a link at that name is followed, like any read) and written only when a setting changes, where a refusal is caught and warned as `settings not saved` — no onboarding depends on it, so it never fails a start. +- **What may be written where.** `flybot-social.json`, the `.bak` copy `startSocial` keeps for another server (`flybot-social.json.`) and `flybot-social-settings.json` are written by one function, and it asks the leaf with `lstat` before the first byte: a missing file is created (mode 600), a plain file of its own (`nlink === 1`) is rewritten, and a symbolic link, a hard link, a directory or any other kind refuses the write — nothing is written there, the file behind a link keeps its bytes, one line names the path and carries no value from the file, and `startSocial` fails with that refusal instead of running on a link. The refusal is asked **before** onboarding: the onboarding is a real home on the server and a round-trip, so a start whose credentials path cannot be written refuses first and never spends one on a token it would drop. A refused `.bak` leaves the credentials in use untouched, and its name is asked in that same pre-check: the backup name follows from the server in the credentials file already on disk (`flybot-social.json.`), a local read, so a link left there refuses the start before the onboarding rather than after it, and a free name still keeps the old credentials under their server. `flybot-social-settings.json` is a different shape and stays as it is: it is read (a link at that name is followed, like any read) and written only when a setting changes, where a refusal is caught and warned as `settings not saved` — no onboarding depends on it, so it never fails a start. A read-only window (`flybot.readOnly?.()`) writes it not at all (#2231): its read endpoints (`social/remote-bots`, `social/friends`) still derive the one-shot inbound backfill, the hidden-remote aging and the accept-notice cleanup in memory and answer the same, but the save path returns without writing, so it cannot create the file, cannot age a hide on it, and cannot flush an older copy over what the writable window saved. - **Validation.** `parseServerUrl`: `https://` only, `http://` just for `localhost`, `127.0.0.1` and `[::1]`; no user name or password, query or fragment; at most 200 characters; an address without a scheme gets `https://` (`http://` for those local hosts). `probeServer` GETs `/v1/platform` without credentials, 8 s deadline, `redirect: 'manual'` (a redirect is refused, so the saved address is the checked one); the answer must look like Bot Social (a `name`, a `platform_key` or a `features` list) and its `features` are shown (cleaned, at most 40) in words (`featureLine` in `client.js`: `typed_by_owner` → typing as you, `mentions` → @mentions, `presence` → online status; an unknown id is only counted, and every raw id is in the line's tooltip, #421). `social/server-set` repeats both before it touches anything, so a UI that skipped **Check** gains nothing. - **A server is an identity.** The default server keeps its files. Any other server gets `flybot-social.json.` (credentials; the same suffix `startSocial` already used for backups, `tag` = first 8 hex of the sha-256 of the normalized address) and `flybot-social-settings..json`; a different server never reads another's file, and no credentials file is ever deleted or rewritten by a switch. The per-Bot parts that live in host state (the `member` identity, the `autoApprove` marks and the person/room grants, which are Bot Social ids and handles) are parked in `flybot-social-bots..json` (no token) and cleared before the new server starts, so a stale grant to `@bob.home` can never apply to a different Bob on the new server; switching back restores them and the saved credentials, without a second onboarding. The old server's friends' Bots leave the list when the new server's first roster fetch replaces it (`remote.publish`); face-to-face rooms already hosted stay as local transcripts, but nothing polls them. - **Applied live, with a way back.** `social/server-set` (needs `confirm: true`; a write, so a read-only host and a multi-member team refuse it) stops the social layer, parks the old server's Bot identities, saves the choice, restores what the new server had, and starts again (first start onboards). If that start fails or takes over 45 s, the previous server's files, identities and choice are put back, it is started again, and the error says so; a start that finishes late is disposed. A disposed instance never re-arms its poll: a face poll already in flight when `dispose()` runs cannot install a new interval on the discarded layer, so it never keeps polling (or delivering from) the server it was built for. When the switch succeeds and the address really changed, the previous server's hosted face-to-face rooms — open or still forming (candidate) — are marked `left` here — a read-only past room whose composer is gone, since the new server cannot reach them. Renaming this home on the same server is not a switch: that server still reaches those rooms, so only a new address ends them (the room and its composer stay as they were). Switching back does not reopen them: a room this home left stays a past room (`faceSnapshotPast`, #182), and a line to it is refused (`bot/room-ended`). The Settings page says plainly that this is a new identity, that friends and sharing stay on the old server, and that the old sign-in is kept as a backup and never deleted. Tests: `plugins/social/test/social.test.js` (`Settings server`, `server controller`, `parseServerUrl`, `probeServer`). diff --git a/plugins/social/src/social.js b/plugins/social/src/social.js index 949e32106..95b9859f8 100644 --- a/plugins/social/src/social.js +++ b/plugins/social/src/social.js @@ -680,7 +680,14 @@ export function createSocial(service, config = {}, { client: given, api: givenAp settings.hiddenRemotes ??= {} settings.inboundBots ??= {} settings.roomBots ??= {} - const saveSettings = () => { if (settingsPath) { try { writeCredentials(settingsPath, settings) } catch (error) { warn('settings not saved', error) } } } + // A read-only window keeps the derivation below but not the write (#2231): `social/remote-bots` + // and `social/friends` are reads, and the backfill, the hidden-remote aging and the accept-notice + // cleanup they reach used to rewrite the file the writable window owns. The seven other + // `flybot.readOnly?.()` sites in this file already drop their writes; this is the one save path. + const saveSettings = () => { + if (flybot.readOnly?.()) return + if (settingsPath) { try { writeCredentials(settingsPath, settings) } catch (error) { warn('settings not saved', error) } } + } // Unset: off. config.autoAccept: true makes it opt-out instead. New friends start in hold (#140). // Config keys may be written with or without '@' and in any case. const approvalHomes = Object.fromEntries(Object.entries(config.approval?.homes ?? {}).map(([key, value]) => [norm(key), value])) diff --git a/plugins/social/test/social.test.js b/plugins/social/test/social.test.js index 6a248eeaa..6cf5e8a32 100644 --- a/plugins/social/test/social.test.js +++ b/plugins/social/test/social.test.js @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { runInNewContext } from 'node:vm' @@ -14154,3 +14154,89 @@ test('#2219: a writable window still opens a past room by creating its mirror', assert.equal(ctx.flybot.rooms.list().length, before + 1, 'a writable window creates the mirror as today') assert.ok(opened?.roomId, 'the new local room is handed back') }) + +// #2231: `social/remote-bots` and `social/friends` are declared reads, and in a read-only window they +// still reached `saveSettings`: the one-shot inbound backfill, the hidden-remote aging and the +// accept-notice cleanup rewrote flybot-social-settings.json, so a window the host had just told +// "close the other one and reload" kept editing the file the writable window owns. The save path is +// the only place #2231 guards; the seven other `flybot.readOnly?.()` sites in social.js already drop +// their writes. These fixtures keep their settings file in their own directory, so "did it write?" +// is answered by the file itself and not by the team's home. +function readOnlyFlybot(flybot) { + return new Proxy(flybot, { get: (target, key) => (key === 'readOnly' ? () => true : key === 'scope' ? (name) => new Proxy(target.scope(name), { get: (inner, k) => (k === 'readOnly' ? () => true : inner[k]) }) : target[key]) }) +} + +async function settingsWindow(t, { settingsPath, readOnly = false }) { + const { ctx, call, stop } = await team() + t.after(stop) + const server = mockServer() + const social = createSocial(readOnly ? readOnlyFlybot(ctx.flybot) : ctx.flybot, { autoAccept: false, inboxPollMs: 0, settingsPath }, { + client: createMcpClient({ server: 'https://s', token: 'owner-token', fetch: server.fetch }), + home: { id: 'h1', handle: '@danyang.home' }, + }) + t.after(() => social.dispose()) + await social.registerAll() + return { ctx, call, server } +} + +const settingsSha = path => createHash('sha256').update(readFileSync(path)).digest('hex') + +test('#2231: a read-only window writes no settings file and still answers its reads', async (t) => { + const dir = mkdtempSync(join(tmpdir(), 'flybot-social-2231-a-')) + t.after(() => rmSync(dir, { recursive: true, force: true })) + const settingsPath = join(dir, 'flybot-social-settings.json') + const { call } = await settingsWindow(t, { settingsPath, readOnly: true }) + const remote = (await call('flybot/social/remote-bots', { fresh: true })).value + assert.equal(existsSync(settingsPath), false, 'a read-only read creates no settings file') + assert.deepEqual(remote.homes[0].bots.map(bot => bot.handle), ['@coder.bob.home', '@writer.bob.home'], 'the roster still comes back') + const friends = (await call('flybot/social/friends', { fresh: true })).value + assert.deepEqual(friends.map(bot => bot.handle), ['@coder.bob.home', '@writer.bob.home'], 'social/friends still answers') +}) + +test('#2231: a read-only window neither ages a hidden remote nor writes a stale copy back', async (t) => { + const dir = mkdtempSync(join(tmpdir(), 'flybot-social-2231-b-')) + t.after(() => rmSync(dir, { recursive: true, force: true })) + const settingsPath = join(dir, 'flybot-social-settings.json') + writeFileSync(settingsPath, JSON.stringify({ autoAccept: {}, via: {}, inboundBots: {}, inboundBackfilled: true, hiddenRemotes: { '@gone.bob.home': { absentFetches: 0 } } })) + const before = settingsSha(settingsPath) + // Both windows read the file before either fetches, the way two open windows do; A then fetches + // twice (it would forget the hide) and B fetches once from its older copy (it would put it back). + const a = await settingsWindow(t, { settingsPath, readOnly: true }) + const b = await settingsWindow(t, { settingsPath, readOnly: true }) + await a.call('flybot/social/remote-bots', { fresh: true }) + await a.call('flybot/social/remote-bots', { fresh: true }) + await b.call('flybot/social/remote-bots', { fresh: true }) + const saved = JSON.parse(readFileSync(settingsPath, 'utf8')) + assert.equal(saved.hiddenRemotes['@gone.bob.home'].absentFetches, 0, 'two read-only reads do not age the hide') + assert.equal(settingsSha(settingsPath), before, 'window B does not write its stale copy back over window A') +}) + +test('#2231: a writable window still persists the backfill its read derives', async (t) => { + const dir = mkdtempSync(join(tmpdir(), 'flybot-social-2231-c-')) + t.after(() => rmSync(dir, { recursive: true, force: true })) + const settingsPath = join(dir, 'flybot-social-settings.json') + const { call } = await settingsWindow(t, { settingsPath }) + await call('flybot/social/remote-bots', { fresh: true }) + const saved = JSON.parse(readFileSync(settingsPath, 'utf8')) + assert.equal(saved.inboundBackfilled, true, 'the one-shot backfill still persists') + assert.deepEqual(Object.keys(saved.inboundBots ?? {}).sort(), ['@coder.bob.home', '@writer.bob.home'], 'the roster is seeded as inbound') +}) + +test('#2231: a writable window still ages a hidden remote and drops a settled accept notice', async (t) => { + const dir = mkdtempSync(join(tmpdir(), 'flybot-social-2231-d-')) + t.after(() => rmSync(dir, { recursive: true, force: true })) + const settingsPath = join(dir, 'flybot-social-settings.json') + writeFileSync(settingsPath, JSON.stringify({ + autoAccept: {}, via: {}, inboundBots: {}, inboundBackfilled: true, + hiddenRemotes: { '@gone.bob.home': { absentFetches: 1 } }, + acceptNotices: [{ id: 'n1', botId: 'a', from: '@coder.bob.home', botName: 'Coder' }, { id: 'n2', botId: 'a', from: '@nobody.x.home', botName: 'Nobody' }], + })) + const { call, server } = await settingsWindow(t, { settingsPath }) + // A Bot that joined our invite is `replyOnly: false` on the roster; only that is evidence the peer + // shared a Bot back, so the notice for it is settled (#1054). + server.tools.list_remote_bots = () => ({ homes: [{ relationship_id: 'r1', home: { bot_id: 'h2', handle: '@bob.home', display_name: 'Bob' }, bots: [{ bot_id: 'x1', handle: '@coder.bob.home', display_name: 'Coder', replyOnly: false }] }] }) + await call('flybot/social/remote-bots', { fresh: true }) + const saved = JSON.parse(readFileSync(settingsPath, 'utf8')) + assert.equal(saved.hiddenRemotes?.['@gone.bob.home'], undefined, 'a second absence still forgets the hide') + assert.deepEqual(saved.acceptNotices.map(notice => notice.id), ['n2'], 'the notice whose peer shared a Bot back is still dropped') +})