Write Bot Social state files atomically and refuse damaged ones

changelog.d/2631.fixed.md

@@ -0,0 +1 @@
+A Bot Social credentials, backup or settings file that exists but no longer parses is now refused with a message naming it instead of being read as "not configured", so a torn file no longer makes a restart register a new home identity and overwrite the old one. Writes land through a temp file and a rename, so a failed write leaves the previous file whole ([#2631](https://github.com/ThinkFlowLab/FlyBot/issues/2631)).

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.<pid>.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.<tag>`) 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.<tag>`), 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.
+- **What may be written where.** `flybot-social.json`, the `.bak` copy `startSocial` keeps for another server (`flybot-social.json.<tag>`) 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 through a same-directory `<path>.<pid>.tmp` that is fsynced and renamed over the file (the temp name is a leaf of its own too, so a link left there refuses the save as well), 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 temp plus rename is what keeps a write that stops half way (ENOSPC, EFBIG, a kill) from taking the previous file with it: the old bytes stay, and a failed write removes its temp (#2631). 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.<tag>`), 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. The read side tells "nothing saved yet" from "damaged": `ENOENT` is a first start, while a file that exists and does not parse refuses with a message naming the path and is left where it is — a torn credentials file therefore never reads as "not configured", and `startSocial` never onboards a fresh home over an identity it could not read (#2631); `flybot-social-settings.json` is read with that same rule (a link at that name is followed, like any read) and written only when a setting changes, where a write refusal is caught and warned as `settings not saved` — no onboarding depends on that write, but a damaged settings file refuses the start instead of reading as empty. 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 `<server>/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.<tag>` (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.<tag>.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.<tag>.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`).

plugins/social/src/social.js

@@ -27,7 +27,7 @@
 // - Backward compat: without a home, config.botId + token use the per-bot identity
 //   (start_conversation / send_message) as before.
 import { createHash } from 'node:crypto'
-import { chmodSync, lstatSync, mkdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from 'node:fs'
+import { chmodSync, closeSync, fsyncSync, lstatSync, mkdirSync, openSync, readFileSync, realpathSync, renameSync, rmSync, statSync, writeFileSync } from 'node:fs'
 import { readFile } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import { basename, dirname, extname, isAbsolute, join, sep } from 'node:path'
@@ -110,7 +110,7 @@
 }
 
 // Remote strings that reach model-visible text: one line, no brackets or markup, capped.
 export const cleanText = (value, fallback = '') => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/[[\]"<>`]/g, '').replace(/\s+/g, ' ').trim().slice(0, 60) || fallback
 // Owner-only draft body: keep punctuation the friend will see; strip controls and cap.
 export const draftText = (value) => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/\s+/g, ' ').trim().slice(0, 800)
 // Owner cards never show the FLYBOT-FACE-HUMAN wire envelope (#159).
@@ -477,9 +477,21 @@
   return () => { for (const off of offs.splice(0).reverse()) off?.() }
 }
 
-export const readCredentials = (path) => {
-  try { return JSON.parse(readFileSync(path, 'utf8')) } catch { return undefined }
+// A state file is read for its bytes only. `ENOENT` is the one answer that means "nothing saved
+// yet"; a file that exists and cannot be read or parsed is not that answer. Reading it as absent is
+// how a start onboarded a fresh home over a torn credentials file (#2631), so it refuses, names the
+// path, and leaves the file where an operator can move it aside on purpose.
+const readStateFile = (path, what) => {
+  let text
+  try { text = readFileSync(path, 'utf8') } catch (error) {
+    if (error?.code === 'ENOENT') return undefined
+    throw Object.assign(new Error(`Bot Social ${what} at ${path} cannot be read: ${error?.message ?? error}`), { code: 'bot/refused', cause: error })
+  }
+  try { return JSON.parse(text) } catch {
+    throw Object.assign(new Error(`Bot Social ${what} at ${path} is damaged: the file exists but does not parse as JSON. Move it aside to start over; nothing here overwrote it.`), { code: 'bot/refused' })
+  }
 }
+export const readCredentials = path => readStateFile(path, 'credentials')
 // The credentials file is one leaf of its own, and so are the `.bak` copy kept for another server
 // and `flybot-social-settings.json`: all three go through this one function. `lstat` does not follow
 // the last component, so a link left at the path reports itself and is refused before the first
@@ -506,7 +518,24 @@
 export const writeCredentials = (path, value) => {
   if (!leafWritable(path)) throw refuseLeaf(path)
   mkdirSync(dirname(path), { recursive: true })
-  writeFileSync(path, JSON.stringify(value, null, 2), { mode: 0o600 })
+  // Same-directory temp, fsync, rename — the shape `server-config.js` writes with. Writing in place
+  // truncated the file first, so a write that stopped half way (ENOSPC, EFBIG, a kill) took the
+  // previous credentials with it (#2631). The temp name is a leaf of its own (temp-name sweep): a
+  // link left there refuses the write instead of taking the bytes outside the folder and renaming
+  // the link into place.
+  const temp = `${path}.${process.pid}.tmp`
+  if (!leafWritable(temp)) throw refuseLeaf(temp)
+  try {
+    const fd = openSync(temp, 'w', 0o600)
+    try {
+      writeFileSync(fd, JSON.stringify(value, null, 2))
+      fsyncSync(fd)
+    } finally { closeSync(fd) }
+    renameSync(temp, path)
+  } catch (error) {
+    try { rmSync(temp, { force: true }) } catch { /* already gone */ }
+    throw error
+  }
   try { chmodSync(path, 0o600) } catch { console.warn('flybot-social:', 'credentials mode not tightened:', path) }
 }
 
@@ -684,7 +713,9 @@
   // identity carries the user's own chat with that friend's Bot.
   const settingsPath = config.settingsPath ?? (config.credentialsPath ? join(dirname(config.credentialsPath), 'flybot-social-settings.json') : undefined)
   let settings = { autoAccept: {}, via: {}, hiddenRemotes: {}, inboundBots: {}, roomBots: {} }
-  try { if (settingsPath) settings = { autoAccept: {}, via: {}, hiddenRemotes: {}, inboundBots: {}, roomBots: {}, ...JSON.parse(readFileSync(settingsPath, 'utf8')) } } catch { /* none yet */ }
+  // Absent is a normal first start; a file that exists and does not parse refuses the start
+  // (#2631) instead of silently dropping the owner's Auto-send, hides and cursors for an empty set.
+  if (settingsPath) settings = { ...settings, ...readStateFile(settingsPath, 'settings') }
   settings.hiddenRemotes ??= {}
   settings.inboundBots ??= {}
   settings.roomBots ??= {}