From e1388e769579493d40610cb4d89639440af8a99d Mon Sep 17 00:00:00 2001 From: nicolas de Barquin Date: Tue, 3 Nov 2026 14:33:36 +0100 Subject: [PATCH 1/2] account-admin: list ids first, details after (lock expiry killed the server) entries('account') streams the account container under its lock; looking up logins and pods inside that loop waited on the same lock until it expired after 6 s and CSS exited. Seen in CI on #36. Co-Authored-By: Claude Opus 5.5 --- account-admin/src/AccountAdminHandler.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/account-admin/src/AccountAdminHandler.ts b/account-admin/src/AccountAdminHandler.ts index 85c2ddd..1cb53a6 100644 --- a/account-admin/src/AccountAdminHandler.ts +++ b/account-admin/src/AccountAdminHandler.ts @@ -142,9 +142,18 @@ export class AccountAdminHandler extends JsonInteractionHandler { } private async list(): Promise { - const accounts: AccountSummary[] = []; + // Ids first, details after. entries() streams the account container + // (JsonResourceStorage) while CSS holds its lock; looking up logins and + // pods inside that loop waited on the same lock, which expired after 6 s + // and took the whole server down ("Lock expired after 6000ms", first CI + // run of the admin tool, 2026-11-03). + const ids: string[] = []; for await (const account of this.accountStorage.entries('account')) { - accounts.push(await this.summary(account.id)); + ids.push(account.id); + } + const accounts: AccountSummary[] = []; + for (const id of ids) { + accounts.push(await this.summary(id)); } return accounts; } From 1f2fb893b894ccce1a3921865371bff68f9241d6 Mon Sep 17 00:00:00 2001 From: nicolas de Barquin Date: Tue, 3 Nov 2026 14:39:39 +0100 Subject: [PATCH 2/2] account-admin: list accounts from the accounts folder, not entries() entries('account') walks every document under /.internal/ through the resource store, lock files included, and stalled until "Lock expired after 6000ms" took the server down (CI on #36, again on #37's first attempt). List the names in .internal/accounts/data/ through the data accessor (directory read, no lock), confirm each with has('account'). docs/admin/explanation.md: "What building it taught us", the findings as arguments for the feature. Co-Authored-By: Claude Opus 5.5 --- account-admin/src/AccountAdminHandler.ts | 34 +++++++++--- .../test/AccountAdminHandler.test.js | 20 ++++--- docs/admin/explanation.md | 54 +++++++++++++++++++ start/admins.js | 1 + 4 files changed, 96 insertions(+), 13 deletions(-) diff --git a/account-admin/src/AccountAdminHandler.ts b/account-admin/src/AccountAdminHandler.ts index 1cb53a6..793c9fb 100644 --- a/account-admin/src/AccountAdminHandler.ts +++ b/account-admin/src/AccountAdminHandler.ts @@ -83,6 +83,7 @@ export class AccountAdminHandler extends JsonInteractionHandler { private readonly auxiliaryStrategy: AuxiliaryStrategy; private readonly locker: ReadWriteLocker; private readonly token: Buffer; + private readonly accountsContainer: ResourceIdentifier; /** * @param accountStorage - The account storage (urn:solid-server:default:AccountStorage). @@ -94,6 +95,7 @@ export class AccountAdminHandler extends JsonInteractionHandler { * @param auxiliaryStrategy - Tells which children the store removes with their subject. * @param locker - The resource locker the store uses. * @param token - The secret a caller must send. Shorter than 32 characters: every call is refused. + * @param baseUrl - The server's base URL, where CSS keeps its internal storage (`.internal/`). */ public constructor( accountStorage: AccountLoginStorage>, @@ -105,6 +107,7 @@ export class AccountAdminHandler extends JsonInteractionHandler { auxiliaryStrategy: AuxiliaryStrategy, locker: ReadWriteLocker, token: string, + baseUrl: string, ) { super(); this.accountStorage = accountStorage; @@ -116,6 +119,10 @@ export class AccountAdminHandler extends JsonInteractionHandler { this.auxiliaryStrategy = auxiliaryStrategy; this.locker = locker; this.token = Buffer.from(token ?? ''); + // Where config/storage/key-value/resource-store.json (JsonResourceStorage, + // container /.internal/) and identity/handler/storage/default.json + // (ContainerPathStorage, /accounts/data/) put one document per account. + this.accountsContainer = { path: new URL('.internal/accounts/data/', baseUrl).href }; } public async handle({ json }: JsonInteractionHandlerInput): Promise { @@ -142,14 +149,27 @@ export class AccountAdminHandler extends JsonInteractionHandler { } private async list(): Promise { - // Ids first, details after. entries() streams the account container - // (JsonResourceStorage) while CSS holds its lock; looking up logins and - // pods inside that loop waited on the same lock, which expired after 6 s - // and took the whole server down ("Lock expired after 6000ms", first CI - // run of the admin tool, 2026-11-03). + // NOT accountStorage.entries('account'). That walks every document under + // CSS's /.internal/ through the resource store, the lock files included + // (they live in the same container), and on 7.3.0 it stalled until "Lock + // expired after 6000ms" and the server exited (first CI run of the admin + // tool, 2026-11-03). Stock CSS never calls it while serving requests. + // Instead: the names in the accounts folder, read through the data + // accessor (a directory listing, no lock), each confirmed by the account + // storage itself. const ids: string[] = []; - for await (const account of this.accountStorage.entries('account')) { - ids.push(account.id); + try { + for await (const metadata of this.accessor.getChildren(this.accountsContainer)) { + const id = decodeURIComponent(metadata.identifier.value.slice(this.accountsContainer.path.length)); + if (id && !id.includes('/') && await this.accountStorage.has('account', id)) { + ids.push(id); + } + } + } catch (error: unknown) { + // No account was ever made: the folder does not exist yet. + if (!NotFoundHttpError.isInstance(error)) { + throw error; + } } const accounts: AccountSummary[] = []; for (const id of ids) { diff --git a/account-admin/test/AccountAdminHandler.test.js b/account-admin/test/AccountAdminHandler.test.js index 7b05e0b..9cf9a0f 100644 --- a/account-admin/test/AccountAdminHandler.test.js +++ b/account-admin/test/AccountAdminHandler.test.js @@ -10,6 +10,8 @@ const { AccountAdminHandler } = require('../dist'); const POD = 'https://gone.example.org/'; const TOKEN = 't'.repeat(43); +const BASE = 'https://example.org/'; +const ACCOUNTS = `${BASE}.internal/accounts/data/`; // A pod as the file backend lists it: containers end in '/', .acl files are // listed as children, .meta files are not. @@ -17,6 +19,9 @@ function makeWorld({ failOn, token = TOKEN } = {}) { const tree = { [POD]: [`${POD}.acl`, `${POD}profile/`, `${POD}README`, `${POD}README.acl`], [`${POD}profile/`]: [`${POD}profile/card`], + // The accounts folder as the file backend lists it, plus a stray entry + // the account storage does not know. + [ACCOUNTS]: ['keep', 'target', 'halfway', 'stale'].map((id) => `${ACCOUNTS}${id}`), }; const log = []; const logins = { @@ -29,11 +34,8 @@ function makeWorld({ failOn, token = TOKEN } = {}) { accountStorage: { delete: async (type, id) => { log.push(`account:${type}:${id}`); }, has: async (type, id) => type === 'account' && id in logins, - entries: async function* (type) { - assert.equal(type, 'account'); - for (const id of Object.keys(logins)) { - yield { id, rememberLogin: false }; - } + entries: async function* () { + throw new Error('entries() walks the lock files too and stalls the server: never call it'); }, }, passwordStore: { @@ -77,7 +79,7 @@ function makeWorld({ failOn, token = TOKEN } = {}) { }; world.handler = new AccountAdminHandler( world.accountStorage, world.passwordStore, world.podStore, world.webIdStore, - world.store, world.accessor, world.auxiliaryStrategy, world.locker, token, + world.store, world.accessor, world.auxiliaryStrategy, world.locker, token, BASE, ); return world; } @@ -100,6 +102,12 @@ test('a configured token shorter than 32 characters refuses everyone, even when } }); +test('an empty server (no accounts folder yet) lists nothing', async () => { + const world = makeWorld(); + world.accessor.getChildren = async function* () { throw new NotFoundHttpError(); }; + assert.deepEqual((await call(world, { token: TOKEN, action: 'list' })).json.accounts, []); +}); + test('lists every account with its logins, pods and WebIDs, a halfway sign-up included', async () => { const { json } = await call(makeWorld(), { token: TOKEN, action: 'list' }); assert.deepEqual(json.accounts, [ diff --git a/docs/admin/explanation.md b/docs/admin/explanation.md index 7915164..ce13c59 100644 --- a/docs/admin/explanation.md +++ b/docs/admin/explanation.md @@ -56,6 +56,60 @@ templates, never reaches CSS). That means no browser page for admins. If one is ever wanted, it needs its own answer to "who is this?", and ADR 012 is the place to decide it. +## What building it taught us + +Each of these was found by running the real server, not by reading the docs. +Together they are the case for doing this inside CSS, carefully, and testing +it on every CSS release. + +**1. The pod root cannot be deleted the normal way, and leaving it keeps +the name taken.** +- *Found:* the store answers "Cannot delete a root storage container" (405, + Solid ยง5.4), and pod creation answers 409 "There already is a resource" + while anything is at the address. +- *So:* only deleting through the store would have looked like success, yet + never freed a name. +- *What we do:* the contents go through the store and the root through the + accessor, under the lock (above). +- *Where it shows:* the smoke run takes the same pod name again after every + deletion. + +**2. CSS's own "list everything" stalls, and a stalled lock takes the whole +server down.** +- *Found (first CI run of the tool, 2026-11-03):* + - Listing accounts with the account storage's `entries('account')` walks + every document CSS keeps under `/.internal/` through the resource store. + That includes the server's lock files, which live in the same place. + - The request waited on a lock until `Lock expired after 6000ms`. Then CSS + exited, taking every pod on that server offline. + - Nothing in stock CSS calls that walk while serving requests, so nobody had + hit it. +- *What we do:* the tool lists the accounts folder's names through the data + accessor (a directory read, no lock). It checks each one against the account + storage, and only then reads that account's logins and pods. +- *Two lessons:* + - **One wrong storage call in a component is an outage, not an error + message.** The smoke run must exercise every route on a real server, on + both pod layouts, at every CSS bump. + - **A script on the data folder would be worse still.** It would hit the + same locks from outside the process that owns them. ("Why inside CSS", + above.) + +**3. An email is not an identity in CSS.** +- *Found:* CSS never verifies that a login's email belongs to whoever signs up + with it. +- *So:* an admin list of emails would make whoever claims an unused address an + admin. +- *What we do:* the permission is the server's terminal ("Why the terminal", + above). + +**4. Stock CSS cannot answer "who has an account here?"** +- *Found:* its account API only shows the signed-in account its own data. +- *So:* without this tool an admin can neither see nor clean what accumulates: + test accounts, sign-ups left halfway, accounts nobody uses. +- *Why it matters:* knowing what lives on the server comes before any + responsible deletion, and before opening a server to the public. + ## Why the connector clean-up is in the script The MCP connector keeps its agents' credentials in `identities.json`, outside diff --git a/start/admins.js b/start/admins.js index 262fbbe..3594cec 100644 --- a/start/admins.js +++ b/start/admins.js @@ -53,6 +53,7 @@ const config = { auxiliaryStrategy: { "@id": "urn:solid-server:default:AuxiliaryStrategy" }, locker: { "@id": "urn:solid-server:default:ResourceLocker" }, token, + baseUrl: { "@id": "urn:solid-server:default:variable:baseUrl" }, }, }, },