Review fixes: holds and create agree, and no password leaves a check

create re-enables what holds refuses (mssql login, mosquitto client,
mailu mailbox, gitea user) and clears an expired postgres password, so
no disabled account loops. mssql and mongodb checks take the password
from the environment, never argv; mosquitto_ctrl failures no longer
repeat -P. mosquitto reads 'could not ask' as an error, not absence.
mailu checks existence and enabled only: its imap passdb cannot verify
a password. mssql checks the user's SID; gitea pages teams at 50.

modules/gitea/client.ts

@@ -390,7 +390,7 @@ export class GiteaAdmin {
   }
 
   private async findTeam(org: string, team: string): Promise<number | null> {
-    const res = await this.request(`/orgs/${encodeURIComponent(org)}/teams`);
+    const res = await this.request(`/orgs/${encodeURIComponent(org)}/teams?limit=50`);
     if (res.status !== 200) return null;
     const match = (res.body as any[] | null)?.find((t) => t?.name === team);
     return match ? Number(match.id) : null;
@@ -414,7 +414,9 @@ export class GiteaAdmin {
         const patch = await this.request(`/admin/users/${encodeURIComponent(username)}`, {
           method: "PATCH",
           // login_name is required by the admin edit endpoint; for a local user it is the username.
-          body: JSON.stringify({ login_name: username, password, must_change_password: false }),
+          // active and prohibit_login: a deactivated or login-prohibited user is refused like a wrong
+          // password, so the provisioner's check reports it lost; applying again must undo both.
+          body: JSON.stringify({ login_name: username, password, must_change_password: false, active: true, prohibit_login: false }),
         });
         if (patch.status === 200) return;
         GiteaAdmin.fail(`/admin/users/${username}`, patch);
@@ -445,7 +447,7 @@ export class GiteaAdmin {
     });
     if (me.status === 401 || me.status === 403) return false;
     if (me.status !== 200) throw new Error(`Gitea GET /user as ${username}: ${me.status}`);
-    const teams = await this.request(`/orgs/${encodeURIComponent(org)}/teams`);
+    const teams = await this.request(`/orgs/${encodeURIComponent(org)}/teams?limit=50`);
     if (teams.status === 404) return false;
     if (teams.status !== 200) GiteaAdmin.fail(`/orgs/${org}/teams`, teams);
     const found = (teams.body as { id: number; name: string }[]).find((t) => t.name === team);

modules/mailu/client.ts

@@ -127,7 +127,9 @@ export class MailuClient {
   }
 
   async changePassword(email: string, password: string): Promise<void> {
-    await this.api("PATCH", `/user/${encodeURIComponent(email)}`, { raw_password: password });
+    // enabled: a disabled mailbox is what the provisioner's check reports as lost, so applying the
+    // mesh's password again also enables it; otherwise the two would disagree for ever.
+    await this.api("PATCH", `/user/${encodeURIComponent(email)}`, { raw_password: password, enabled: true });
   }
 
   async deleteUser(email: string): Promise<void> {
@@ -135,34 +137,24 @@ export class MailuClient {
   }
 
   /**
-   * Whether a mailbox exists, is enabled, and accepts exactly this password. Read-only. Existence
-   * from the admin API; the password from `doveadm auth test` in the imap container, which is how
-   * the mail server itself authenticates, and which exits 77 for a refused login. The password
-   * reaches doveadm through the exec's environment, never the host's argv. An unreachable API or
-   * container rejects (novox/hq issue 120).
+   * Whether a mailbox exists and is enabled. Read-only, through the admin API.
+   *
+   * **The password is not checked.** Mailu authenticates in its admin service, behind the front;
+   * the imap server's own password database accepts any password from Mailu's subnet, so asking it
+   * (`doveadm auth test`) proves nothing, or refuses everyone. A lost or disabled mailbox is caught;
+   * a password changed by hand is not (novox/hq issue 120).
    */
-  async holdsUser(email: string, password: string): Promise<boolean> {
+  async holdsUser(email: string): Promise<boolean> {
     const res = await fetch(`${this.baseUrl}/user/${encodeURIComponent(email)}`, {
       headers: { Authorization: this.apiKey, Accept: "application/json" },
     });
     if (res.status === 404) return false;
     if (!res.ok) throw new Error(`Mailu API GET /user/${email}: ${res.status} ${await res.text()}`);
     const user = (await res.json()) as { enabled?: boolean };
-    if (user.enabled === false) return false;
-    try {
-      await run(
-        "docker",
-        ["exec", "-e", "MESH_USER", "-e", "MESH_PW", this.imapContainer,
-          "sh", "-c", 'doveadm auth test "$MESH_USER" "$MESH_PW"'],
-        { env: { ...process.env, MESH_USER: email, MESH_PW: password }, timeout: 30_000 },
-      );
-      return true;
-    } catch (err) {
-      if ((err as { code?: number }).code === 77) return false;
-      throw err;
-    }
+    return user.enabled !== false;
   }
 
+
   async listAliases(): Promise<MailuAlias[]> {
     const aliases = await this.api<any[]>("GET", "/alias");
     return (aliases ?? []).map((a) => ({

modules/mailu/provisioner/index.ts

@@ -65,6 +65,6 @@ runProvisioner("smtp", {
   // Asked every minute by the harness: whether the backend still holds this consumer exactly as
   // the mesh gave it, so a login lost behind the provisioner's back is made again (novox/hq issue 120).
   async holds(p: Provision): Promise<boolean> {
-    return mailu.holdsUser(addressOf(p), p.password);
+    return mailu.holdsUser(addressOf(p));
   },
 });

modules/mongodb/client.ts

@@ -115,21 +115,23 @@ print(EJSON.stringify({ ok: 1 }));
    * authentication failure or a missing role; an unreachable server rejects (novox/hq issue 120).
    */
   async canAuthenticateAs(database: string, user: string, password: string): Promise<boolean> {
-    const uri =
-      `mongodb://${encodeURIComponent(user)}:${encodeURIComponent(password)}@${this.conn.host}:${this.conn.port}` +
-      `/${encodeURIComponent(database)}?authSource=${encodeURIComponent(database)}&serverSelectionTimeoutMS=10000`;
+    // Connected without credentials, then authenticated inside the eval from the environment, so
+    // the consumer's password is neither on argv nor in the message of a failed command.
+    const uri = `mongodb://${this.conn.host}:${this.conn.port}/?serverSelectionTimeoutMS=10000`;
+    const js =
+      "const t = db.getSiblingDB(process.env.MESH_HOLDS_DB);" +
+      "t.auth(process.env.MESH_HOLDS_USER, process.env.MESH_HOLDS_PW);" +
+      "print(EJSON.stringify(t.runCommand({ connectionStatus: 1 }).authInfo.authenticatedUserRoles))";
     let stdout: string;
     try {
-      ({ stdout } = await run(
-        "mongosh",
-        [uri, "--quiet", "--eval",
-          "print(EJSON.stringify(db.runCommand({ connectionStatus: 1 }).authInfo.authenticatedUserRoles))"],
-        { timeout: 30_000 },
-      ));
+      ({ stdout } = await run("mongosh", [uri, "--quiet", "--eval", js], {
+        env: { ...process.env, MESH_HOLDS_DB: database, MESH_HOLDS_USER: user, MESH_HOLDS_PW: password },
+        timeout: 30_000,
+      }));
     } catch (err) {
       const text = `${(err as { stderr?: string }).stderr ?? ""}${(err as { stdout?: string }).stdout ?? ""}`;
       if (/Authentication failed|AuthenticationFailed/i.test(text)) return false;
-      throw err;
+      throw new Error(`mongosh could not check ${user}: ${text.trim().slice(0, 500) || String((err as Error).message).split("\n")[0]}`);
     }
     const roles = JSON.parse(stdout.trim()) as { role: string; db: string }[];
     return roles.some((r) => r.role === "dbOwner" && r.db === database);

modules/mosquitto/client.ts

@@ -88,9 +88,20 @@ export class MosquittoClient {
       "-u", this.conn.adminUser,
       "-P", this.conn.adminPassword,
     ];
-    const { stdout, stderr } = await run("mosquitto_ctrl", [...base, "dynsec", ...args], {
-      maxBuffer: 16 << 20,
-    });
+    let stdout: string;
+    let stderr: string;
+    try {
+      ({ stdout, stderr } = await run("mosquitto_ctrl", [...base, "dynsec", ...args], {
+        maxBuffer: 16 << 20,
+        timeout: 30_000,
+      }));
+    } catch (err) {
+      // A failed run's message repeats its argv, the admin password (-P) included; say what failed
+      // without it.
+      const e = err as { code?: unknown; signal?: unknown; stderr?: string; stdout?: string };
+      const detail = `${e.stderr ?? ""}${e.stdout ?? ""}`.trim().slice(0, 500);
+      throw new Error(`mosquitto_ctrl dynsec ${args[0] ?? ""} could not run (${e.code ?? e.signal ?? "error"}): ${detail}`);
+    }
     const failure = ctlError(`${stdout}\n${stderr}`);
     if (failure) {
       throw new Error(`mosquitto_ctrl dynsec ${args[0] ?? ""} failed: ${failure}`);
@@ -141,6 +152,11 @@ export class MosquittoClient {
 
     if (await this.clientExists(username)) {
       await this.ctl("setClientPassword", username, password);
+      // A disabled client is refused like a wrong password, so the check the provisioner runs
+      // reports it lost; applying again must enable it, or the two would disagree for ever.
+      if (/Disabled:\s*true/i.test(await this.ctl("getClient", username))) {
+        await this.ctl("enableClient", username);
+      }
     } else {
       await this.ctl("createClient", username, "-p", password);
     }
@@ -174,7 +190,16 @@ export class MosquittoClient {
     const code = await mqttConnack(this.conn.host, this.conn.port, username, password);
     if (code === 4 || code === 5) return false;
     if (code !== 0) throw new Error(`mosquitto refused ${username} with CONNACK ${code}`);
-    return this.clientHasRole(username, username);
+    // The role, asked directly: only "not found" means absent. Any other failure to ask rejects,
+    // unlike clientHasRole, which reads every failure as "no role".
+    let out: string;
+    try {
+      out = await this.ctl("getClient", username);
+    } catch (err) {
+      if (/not\s*found|does not exist|no such/i.test(String(err))) return false;
+      throw err;
+    }
+    return new RegExp(`(^|\\s)${escapeRegExp(username)}\\s+\\(priority`, "m").test(out);
   }
 
   /** Remove a client and the per-client role created for it, idempotently. */

modules/mssql/client.ts

@@ -75,14 +75,18 @@ export class MssqlClient {
    * prints (split across output lines for a large result, and reassembled here) is parsed. An
    * empty result yields no output at all — an empty array.
    */
-  async query(select: string, database = "master"): Promise<Record<string, unknown>[]> {
+  async query(
+    select: string,
+    database = "master",
+    variables: Record<string, string> = {},
+  ): Promise<Record<string, unknown>[]> {
     const wrapped = `SET NOCOUNT ON;\n${stripTrailingSemis(select)}\nFOR JSON PATH, INCLUDE_NULL_VALUES;`;
-    const stdout = await this.sqlcmd(wrapped, database);
+    const stdout = await this.sqlcmd(wrapped, database, variables);
     return parseJsonRows(stdout);
   }
 
   /** The one execution boundary: invoke `sqlcmd` and return its concatenated stdout. */
-  private async sqlcmd(sql: string, database: string): Promise<string> {
+  private async sqlcmd(sql: string, database: string, variables: Record<string, string> = {}): Promise<string> {
     // `-h -1` drops the column-header rule; `-y 0`/`-Y 0` lift the display-width cap so a long
     // JSON document is not truncated; `-W` trims trailing whitespace so the JSON chunks rejoin
     // cleanly. sqlcmd from the mssql-tools ships in the runtime container, the way `psql` ships
@@ -101,7 +105,9 @@ export class MssqlClient {
         "-W",
         "-Q", sql,
       ],
-      { env: { ...process.env, SQLCMDPASSWORD: this.conn.password }, maxBuffer: 16 << 20 },
+      // `variables` reach sqlcmd as environment variables, which it substitutes as `$(NAME)` scripting
+      // variables: a value that must not appear on argv, or in the message of a failed command.
+      { env: { ...process.env, ...variables, SQLCMDPASSWORD: this.conn.password }, maxBuffer: 16 << 20 },
     );
     return stdout;
   }
@@ -121,6 +127,9 @@ export class MssqlClient {
       );
     } else {
       await this.exec(`ALTER LOGIN ${ident(login)} WITH PASSWORD = ${literal(password)}`);
+      // A disabled login is refused like a wrong password; the check the provisioner runs reports it
+      // lost, so applying again must enable it or the two would disagree for ever.
+      await this.exec(`ALTER LOGIN ${ident(login)} ENABLE`);
     }
 
     const dbs = await this.query(
@@ -138,6 +147,10 @@ export class MssqlClient {
     );
     if (users.length === 0) {
       await this.exec(`CREATE USER ${ident(login)} FOR LOGIN ${ident(login)}`, database);
+    } else {
+      // Re-point an existing user at the login. A database restored from elsewhere keeps its user
+      // under the old login's SID, orphaned; this maps it back, and is a no-op when it already is.
+      await this.exec(`ALTER USER ${ident(login)} WITH LOGIN = ${ident(login)}`, database);
     }
     await this.exec(`ALTER ROLE db_owner ADD MEMBER ${ident(login)}`, database);
   }
@@ -148,14 +161,24 @@ export class MssqlClient {
    * nothing logs in and no failed-login is recorded (novox/hq issue 120).
    */
   async holdsLogin(database: string, login: string, password: string): Promise<boolean> {
+    // The password reaches sqlcmd as a scripting variable from the environment, never inside the
+    // query text, so it is neither on argv nor in the message of a failed command. It is the mesh's
+    // minted value, which carries no quote.
     const server = await this.query(
       `SELECT CAST(CASE WHEN EXISTS (SELECT 1 FROM sys.sql_logins WHERE name = ${literal(login)} ` +
-        `AND is_disabled = 0 AND PWDCOMPARE(${literal(password)}, password_hash) = 1) ` +
+        `AND is_disabled = 0 AND PWDCOMPARE(N'$(MESHHOLDSPW)', password_hash) = 1) ` +
         `AND DB_ID(${literal(database)}) IS NOT NULL THEN 1 ELSE 0 END AS int) AS ok`,
+      "master",
+      { MESHHOLDSPW: password },
     );
     if (Number(server[0]?.ok) !== 1) return false;
+    // The user must be this login's, by SID, and a db_owner. A user orphaned by a restore has the
+    // right name and the wrong SID, and cannot be reached through the login.
     const owner = await this.query(
-      `SELECT CAST(IS_ROLEMEMBER('db_owner', ${literal(login)}) AS int) AS ok`,
+      `SELECT CAST(CASE WHEN EXISTS (SELECT 1 FROM sys.database_principals dp ` +
+        `JOIN sys.server_principals sp ON dp.sid = sp.sid ` +
+        `WHERE dp.name = ${literal(login)} AND sp.name = ${literal(login)}) ` +
+        `AND IS_ROLEMEMBER('db_owner', ${literal(login)}) = 1 THEN 1 ELSE 0 END AS int) AS ok`,
       database,
     );
     return Number(owner[0]?.ok) === 1;

modules/postgres/client.ts

@@ -88,9 +88,11 @@ export class PostgresClient {
   async createDatabaseAndRole(database: string, role: string, password: string): Promise<void> {
     const roles = await this.query("SELECT 1 FROM pg_roles WHERE rolname = " + literal(role));
     if (roles.rows.length === 0) {
-      await this.query(`CREATE ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)}`);
+      await this.query(`CREATE ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)} VALID UNTIL 'infinity'`);
     } else {
-      await this.query(`ALTER ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)}`);
+      // VALID UNTIL 'infinity': a password that expired is refused like a wrong one, so the check the
+      // provisioner runs would report it lost, and only clearing the expiry makes applying it again work.
+      await this.query(`ALTER ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)} VALID UNTIL 'infinity'`);
     }
     const dbs = await this.query("SELECT 1 FROM pg_database WHERE datname = " + literal(database));
     if (dbs.rows.length === 0) {