feat(cli): add a read-only doctor command that explains itself (#23)

Co-authored-by: Maurice Schmicking <maurice.schmicking@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: mschmicking

AGENTS.md

@@ -116,6 +116,27 @@ infinite-loops on connection errors against this server.
 - No new npm dependencies without a good reason. Current set: `ws`, `commander`,
   `chokidar`, `diff`.
 
+### Commit and PR titles
+
+Conventional commits, enforced on the **PR title** by `.github/workflows/pr-title.yml`
+— the title becomes the squashed commit message and release-please derives the next
+version from it, so a title that does not parse is a release that silently never
+happens.
+
+Type is one of `feat`, `fix`, `perf`, `refactor`, `docs`, `test`, `build`, `ci`,
+`chore`, `revert`. A scope is optional, **but any scope used must be one of**:
+
+```
+sync  auth  watch  logs  json  types  cli  deps  docs  release  main
+```
+
+That list is closed on purpose, to keep the vocabulary small enough to mean something.
+A new component does **not** earn a new scope — `feat(doctor)` fails the check, and a
+new CLI command belongs under `cli`. If a scope genuinely has to be added, add it to
+the workflow in the same PR. (`main` is not a component: it is the branch name in
+release-please's own `chore(main): release x.y.z` title, and without it the release PR
+cannot merge.) Subjects must not end with a full stop.
+
 ## Commands
 
 ```bash
@@ -134,7 +155,8 @@ Directly tested: `client/socket`, `client/objects`, `client/auth` (HTTP and HTTP
 `config`, `credentials`, `sync/compare`, `sync/mapping`, `sync/safe-path` (path
 traversal, symlink-file writes, symlinked-directory writes), the `--json` record shapes,
 and the commands `pull`, `push`, `status`, `diff` (including `--against`), `watch`,
-`logs`, `list`, `start`, `stop`, `restart`, `new`, `rename`, `move`, `remove`, `backup`.
+`logs`, `list`, `start`, `stop`, `restart`, `new`, `rename`, `move`, `remove`, `backup`,
+`doctor`.
 `sync/manifest` and `sync/scan` are covered indirectly by every pull/push test.
 
 `test/cli.test.ts` spawns the built `dist/cli.js` and asserts on real argv handling.
@@ -146,7 +168,8 @@ there, and it needs `dist/` built first.**
 `test/fake-server.ts` serves HTTP and websocket on one port, as real Admin does on 8081.
 Its `auth` field selects how the HTTP side answers the probes in `client/auth.ts`
 (`disabled` → `GET /login` 404s, `oauth` → `POST /oauth/token`, `legacy` → `POST /login`);
-`reset()` returns it to `disabled`, so tests that do not care are unaffected.
+`reset()` returns it to `disabled`, so tests that do not care are unaffected. Its
+`requireCookieOnSocket` flag reproduces the silent-auth failure below.
 
 `pull` also skips-and-continues per script rather than aborting the whole run: one
 unwritable script (a bad id, a symlink in the way) is reported and the rest still
@@ -207,6 +230,27 @@ always `!allowSelfSigned` — the value is the user's decision, not a constant,
 writing it as one both misreports what the code does and trips CodeQL's
 `js/disabling-certificate-validation`.
 
+### Two failures that look like a broken instance and are not
+
+Both cost a session an hour, and `commands/doctor.ts` exists because of them.
+
+1. **An unauthenticated socket is silent, not angry.** Admin accepts the connection,
+   sends `___ready___`, and then ignores every command. There is no auth error and no
+   close — requests simply never come back, and the only thing the caller ever sees is
+   `Request "..." timed out`. The timeout in `client/socket.ts` therefore carries a
+   hint naming this cause; `test/fake-server.ts` reproduces it via
+   `requireCookieOnSocket`.
+2. **An expired self-signed certificate is harmless here.** With `allowSelfSigned` the
+   identity check is the pinned fingerprint, not the chain, so `iob-sync` keeps working
+   after expiry while every other client on that port fails with
+   `certificate has expired`. `doctor` reports it as OK plus a note. Do not "fix" this
+   by tightening the TLS path — the pin is the check, and it is stricter than the chain.
+
+**`iob-sync` is a CLI, not a library.** `package.json` exposes `bin` only; there is no
+`exports` map and nothing under `src/` is a supported import. Anything constructing
+`AdminSocketClient` directly must reproduce `withContext` in `cli.ts` — certificate
+check, then cookie, then socket — and the failure mode when it does not is (1) above.
+
 ### Two bugs the watch tests caught
 
 Both were live in working code, and both are the kind that only show up under a test
@@ -224,8 +268,6 @@ testable at all, which matters given a regression there means an infinite push l
 
 ## Other known gaps
 
-- Self-signed certificates are honoured on the websocket path but not on the HTTP
-  auth path (would require an `undici` Agent).
 - `init --types` writes the ioBroker type definitions, but the download of
   `javascript.d.ts` from GitHub has only been exercised against a live network.
 
@@ -296,3 +338,10 @@ window so it measures the debounce rather than the disk.
 If it recurs, that file is the suspect and the fix is more timing margin — **not**
 loosening an assertion. The thing being tested is the guard against an infinite push
 loop against someone's house.
+
+One of these turned out not to be timing at all. The `--pull` case waited for the file
+content to appear and then asserted on the log line, but `applyRemote` writes the file,
+saves the manifest and logs **last** — so the assertions could run inside that window.
+It now waits for the `pull` line, which is the operation's real completion signal, and
+asserts on the file and manifest afterwards. Before assuming load, check what the code
+under test does in what order.

README.md

@@ -114,6 +114,7 @@ If `logs` shows nothing, that is usually the adapter's own log level rather than
 | `types`            | Set up editor intellisense (`log`, `schedule`, ...). `--force`, `--offline`.                                                                                            |
 | `login` / `logout` | Save or remove the password for this instance. Never stored in the project.                                                                                             |
 | `trust`            | Accept the instance's current TLS certificate. Only needed after it changes. `--yes` skips the prompt.                                                                  |
+| `doctor`           | Check config, certificate, login, connection and a live round-trip, and say which one is wrong. Read-only, never prompts. Run this first when something looks broken.   |
 | `pull [pattern]`   | Download scripts to disk. Never deletes or overwrites local files.                                                                                                      |
 | `push [pattern]`   | Upload locally modified scripts. Never deletes remote objects.                                                                                                          |
 | `status`           | Show what changed, locally and remotely.                                                                                                                                |

docs/TROUBLESHOOTING.md

@@ -2,6 +2,43 @@
 
 Things that look like bugs and are not. See the [README](../README.md) for the overview.
 
+**Start with `iob-sync doctor`.** It checks the config, the certificate, the login, the
+connection and a live round-trip, and names the one that is wrong — including the two
+cases below, which are the ones that reliably send people down the wrong path. It is
+read-only and never prompts, so it is safe to run at any time.
+
+## Commands time out, but the connection "works"
+
+Symptom: `___ready___` arrives, the socket reports connected, and then every request
+fails with `Request "getObject" timed out after 20000ms.`
+
+The session is not authenticated. ioBroker Admin does not answer an unauthenticated
+command with an error — it accepts the connection, sends `___ready___`, and then
+ignores the command entirely. There is nothing to find in the log, because nothing
+went wrong at the transport layer.
+
+Usually this means the stored password is stale (`iob-sync login` replaces it) or the
+session expired during a long-running `watch`. It also happens to anyone driving
+`AdminSocketClient` from their own code without an auth cookie: **iob-sync is a CLI,
+not a library** — there is no supported import path, and the wiring the commands rely
+on (certificate check, then cookie, then socket) lives in `withContext` in
+`src/cli.ts`. A client constructed without `cookie`, `allowSelfSigned` and
+`certFingerprint` connects perfectly and then does nothing.
+
+## `certificate has expired` from other tools on the same port
+
+`iob-sync` keeps working while every other client refuses to connect. Both are correct.
+
+A home ioBroker signs its own certificate, typically for one year, and nothing renews
+it. With `allowSelfSigned` the chain is not what establishes identity here — the pinned
+SHA-256 fingerprint is (see [AUTHENTICATION.md](AUTHENTICATION.md)) — and an expired
+certificate signs exactly as well as a fresh one. Anything validating the chain the
+normal way rejects it.
+
+`iob-sync doctor` reports this as OK with a note rather than as a fault. To make the
+other tools happy, regenerate the certificate on the instance and then run
+`iob-sync trust` to accept the new fingerprint.
+
 ## `logs` prints the banner and nothing else
 
 It is streaming; there is simply nothing to show. Two things surprise people:

src/cli.ts

@@ -34,6 +34,7 @@ import { move } from './commands/move';
 import { remove } from './commands/remove';
 import { login, logout } from './commands/login';
 import { trust } from './commands/trust';
+import { doctor } from './commands/doctor';
 import { setupTypes } from './commands/types';
 
 /**
@@ -292,6 +293,20 @@ program
     })();
   });
 
+program
+  .command('doctor')
+  .description('check that iob-sync can reach and authenticate to the instance')
+  .option('--timeout <ms>', 'budget for the connect and round-trip probes', '8000')
+  .action(function (this: Command) {
+    const opts = this.opts();
+    return action(async () => {
+      // Not withContext: that connects first and aborts on the first failure, which
+      // is precisely the information this command is here to report.
+      const { root, config } = await loadConfig(resolveCwd());
+      await doctor(root, config, { timeoutMs: Number(opts.timeout) || undefined }, logger);
+    })();
+  });
+
 program
   .command('logout')
   .description('remove the stored password for this instance')

src/client/socket.ts

@@ -32,6 +32,21 @@ const DEFAULT_CONNECT_TIMEOUT_MS = 15000;
 const DEFAULT_REQUEST_TIMEOUT_MS = 20000;
 const CLIENT_NAME = 'iobroker-sync';
 
+/**
+ * Attached to every request timeout, because this is the only thing an unauthenticated
+ * caller ever sees.
+ *
+ * Admin accepts the connection and sends `___ready___` whether or not a session cookie
+ * came with it, and then simply ignores commands — no auth error, no close. A bare
+ * "timed out" therefore reads as a slow or broken instance, which sends people
+ * debugging the wrong thing entirely. Naming the likely cause here costs one line.
+ */
+const TIMEOUT_HINT =
+  'A socket that is open but ignores commands is what an unauthenticated or expired ' +
+  'session looks like — Admin sends no auth error. Run `iob-sync doctor`. If you are ' +
+  'driving AdminSocketClient yourself, it needs `cookie`, `allowSelfSigned` and ' +
+  '`certFingerprint` — see `withContext` in src/cli.ts.';
+
 type Frame = [number, number | null, string?, unknown?];
 
 /**
@@ -289,7 +304,12 @@ export class AdminSocketClient implements SocketClient {
     return new Promise<T>((resolve, reject) => {
       const timer = setTimeout(() => {
         this.pending.delete(id);
-        reject(new UserError(`Request "${command}" timed out after ${this.requestTimeoutMs}ms.`));
+        reject(
+          new UserError(
+            `Request "${command}" timed out after ${this.requestTimeoutMs}ms.`,
+            TIMEOUT_HINT,
+          ),
+        );
       }, this.requestTimeoutMs);
 
       this.pending.set(id, {

src/client/tls.ts

@@ -85,13 +85,54 @@ function hostAndPort(url: string): { host: string; port: number } {
   return { host: parsed.hostname, port: Number(parsed.port) || 443 };
 }
 
+/**
+ * What the server's certificate says about itself.
+ *
+ * The fingerprint is the only field the pin depends on. The rest exists for
+ * `iob-sync doctor`, which has to be able to say *why* a certificate is or is not a
+ * problem — "expired seven months ago, and that is fine here" is a sentence nobody
+ * can write from a fingerprint alone.
+ */
+export interface CertificateInfo {
+  /** SHA-256 fingerprint, colon-separated uppercase hex. */
+  fingerprint: string;
+  /** Distinguished name, flattened for display, e.g. `CN=iobroker`. */
+  subject: string;
+  issuer: string;
+  /** Undefined when the certificate carries a date Node could not parse. */
+  validFrom?: Date;
+  validTo?: Date;
+}
+
+/** `{ CN: 'iobroker', O: 'ioBroker' }` -> `CN=iobroker, O=ioBroker`. */
+function formatDn(dn: tls.PeerCertificate['subject'] | undefined): string {
+  if (!dn || typeof dn !== 'object') return '';
+  return Object.entries(dn)
+    .map(([key, value]) => `${key}=${String(value)}`)
+    .join(', ');
+}
+
+function parseCertDate(raw: string | undefined): Date | undefined {
+  if (!raw) return undefined;
+  const parsed = new Date(raw);
+  return Number.isNaN(parsed.getTime()) ? undefined : parsed;
+}
+
 /**
  * Opens a TLS connection purely to read the certificate, then closes it.
  *
  * Nothing is written to the socket. That is the whole point: this runs before the
  * password is sent, so a certificate the user ends up rejecting never sees it.
  */
-export function probeCertificate(url: string, allowSelfSigned: boolean): Promise<string> {
+export async function probeCertificate(url: string, allowSelfSigned: boolean): Promise<string> {
+  return (await probeCertificateInfo(url, allowSelfSigned)).fingerprint;
+}
+
+/** As `probeCertificate`, but keeps the fields the fingerprint alone cannot explain. */
+export function probeCertificateInfo(
+  url: string,
+  allowSelfSigned: boolean,
+): Promise<CertificateInfo> {
   return new Promise((resolve, reject) => {
     let target: { host: string; port: number };
     try {
@@ -104,11 +145,11 @@ export function probeCertificate(url: string, allowSelfSigned: boolean): Promise
     // The handshake, the timeout and the error handler all race to end this; whichever
     // gets there first closes the socket and the rest become no-ops.
     let settled = false;
-    const succeed = (fingerprint: string): void => {
+    const succeed = (info: CertificateInfo): void => {
       if (settled) return;
       settled = true;
       socket.destroy();
-      resolve(fingerprint);
+      resolve(info);
     };
     const fail = (err: Error): void => {
       if (settled) return;
@@ -138,7 +179,13 @@ export function probeCertificate(url: string, allowSelfSigned: boolean): Promise
           );
           return;
         }
-        succeed(fingerprint.toUpperCase());
+        succeed({
+          fingerprint: fingerprint.toUpperCase(),
+          subject: formatDn(cert.subject),
+          issuer: formatDn(cert.issuer),
+          validFrom: parseCertDate(cert.valid_from),
+          validTo: parseCertDate(cert.valid_to),
+        });
       },
     );