fix: bound the window in which the shared key still works for a worker (#233)

Per-worker fleet keys exist so CASHPILOT_API_KEY stops being enough to speak as
any particular worker. The cutover finalized when a worker first heartbeat with
its own key -- and for a worker that CANNOT persist that key, it never did. A
pre-1.0.0 image, a read-only /data, an ephemeral container: the shared key stayed
valid for that identity forever, the UI re-sent the per-worker key to whoever
held the shared one every 60 seconds, and the only trace was one log line a
minute.

The window now also closes 24 hours after the key is minted. Past it the shared
key is refused for that worker and the fleet page marks it "enrollment
incomplete", with what causes it and how to recover.

Absent is not expired: a missing or unparseable issue time reads as still
enrolling, and the migration backfills existing unconfirmed workers to now, so
this upgrade cannot take a mid-enrollment fleet offline.

docs/upgrade-v1.md claimed an old worker "can no longer heartbeat", which is why
nobody went looking for one still on the shared key. It now says what happens.

Author: GeiserX

app/database.py

@@ -385,6 +385,7 @@ def decrypt_value(value: str) -> str:
     last_heartbeat  TEXT,
     api_key_enc     TEXT,
     key_confirmed   INTEGER NOT NULL DEFAULT 0,
+    key_issued_at   TEXT,
     registered_at   TEXT    NOT NULL DEFAULT (datetime('now'))
 );
 
@@ -740,6 +741,20 @@ async def init_db() -> None:
             await db.execute("ALTER TABLE workers ADD COLUMN api_key_enc TEXT")
         if "key_confirmed" not in cols:
             await db.execute("ALTER TABLE workers ADD COLUMN key_confirmed INTEGER NOT NULL DEFAULT 0")
+        if "key_issued_at" not in cols:
+            # When the per-worker key was minted, so the window in which the
+            # SHARED key still works for that worker can be bounded.
+            await db.execute("ALTER TABLE workers ADD COLUMN key_issued_at TEXT")
+            # Backfilled to NOW, not to NULL and not to the distant past.
+            # Every already-enrolled-but-unconfirmed worker would otherwise be
+            # instantly past its window the moment this upgrade lands, and a
+            # patch release would take a working fleet offline with no warning.
+            # Absent is not expired: these get a full fresh window, and only a
+            # worker that still cannot confirm within it is cut off.
+            await db.execute(
+                "UPDATE workers SET key_issued_at = datetime('now') "
+                "WHERE api_key_enc IS NOT NULL AND api_key_enc != '' AND key_confirmed = 0"
+            )
 
         # Migrate earnings table: add fx_rate_usd so a non-USD balance's value at the
         # time it was recorded stays reconstructable (rates are only cached live).
@@ -1632,7 +1647,7 @@ async def set_worker_key(client_id: str, key: str) -> None:
     db = await _get_db()
     try:
         cursor = await db.execute(
-            "UPDATE workers SET api_key_enc = ?, key_confirmed = 0 WHERE client_id = ?",
+            "UPDATE workers SET api_key_enc = ?, key_confirmed = 0, key_issued_at = datetime('now') WHERE client_id = ?",
             (encrypt_value(key), client_id),
         )
         await db.commit()
@@ -1711,6 +1726,27 @@ async def get_worker_key_state(client_id: str) -> tuple[str | None, bool]:
         await db.close()
 
 
+async def get_worker_key_issued_at(client_id: str) -> str | None:
+    """When this worker's per-worker key was minted, or None if not recorded.
+
+    Separate from ``get_worker_key_state`` rather than widening its tuple: that
+    function is called on every heartbeat and its 2-tuple contract is relied on
+    in several places, while this is only needed on the one branch that decides
+    whether the shared key may still be honoured.
+
+    None means UNKNOWN — a row written before this column existed and missed by
+    the migration's backfill. Callers must not read it as "long ago"; unknown is
+    not expired.
+    """
+    db = await _get_db()
+    try:
+        cursor = await db.execute("SELECT key_issued_at FROM workers WHERE client_id = ?", (client_id,))
+        row = await cursor.fetchone()
+        return row["key_issued_at"] if row else None
+    finally:
+        await db.close()
+
+
 # --- Health Events ---
 
 

app/main.py

@@ -3459,14 +3459,67 @@ def _bearer_token(request: Request) -> str:
     return h[7:] if h.startswith("Bearer ") else ""
 
 
+WORKER_KEY_CONFIRM_WINDOW = timedelta(hours=24)
+
+
+def _humanize_window() -> str:
+    hours = WORKER_KEY_CONFIRM_WINDOW.total_seconds() / 3600
+    return f"more than {hours:g} hours" if hours >= 1 else f"more than {WORKER_KEY_CONFIRM_WINDOW.total_seconds():g}s"
+
+
+def enrollment_state(key_issued_at: str | None, confirmed: bool, now: datetime | None = None) -> str:
+    """``"confirmed"``, ``"pending"`` (still inside the window) or ``"incomplete"``.
+
+    Pure, so the heartbeat guard and the fleet page cannot disagree about which
+    workers are still enrolling and which have stalled.
+
+    An UNPARSEABLE or missing timestamp reads as ``"pending"``. That is the safe
+    direction: reading unknown as expired would lock out a worker on the
+    strength of a value nobody wrote.
+    """
+    if confirmed:
+        return "confirmed"
+    if not key_issued_at:
+        return "pending"
+    try:
+        issued = datetime.fromisoformat(key_issued_at).replace(tzinfo=UTC)
+    except (TypeError, ValueError):
+        logger.warning(
+            "Worker key issue time %r is unparseable; treating enrollment as still in progress", key_issued_at
+        )
+        return "pending"
+    return "pending" if (now or datetime.now(UTC)) - issued <= WORKER_KEY_CONFIRM_WINDOW else "incomplete"
+
+
+async def _enrollment_window_open(cid: str) -> bool:
+    """Whether the shared key may still be honoured for this enrolled worker.
+
+    A second read of the same row, rather than widening ``get_worker_key_state``.
+    It is reached only on the rare branch — a worker that has a key, has never
+    used it, and is presenting the shared key. Every steady-state heartbeat
+    returns "ok" before this line, so this is not on the hot path.
+    """
+    return enrollment_state(await database.get_worker_key_issued_at(cid), confirmed=False) == "pending"
+
+
 async def _authenticate_worker_heartbeat(request: Request, cid: str) -> str:
     """Authenticate a heartbeat and classify it. Returns one of:
 
     - ``"enroll"``  — worker has no key yet and presented the shared key: mint one.
     - ``"reissue"`` — worker has a key that is NOT yet confirmed and presented the
       shared key: it likely lost the enrollment response, so re-deliver the SAME
-      key (until confirmed, the shared key still works for this one worker — a
-      bounded window that closes on the worker's first own-key heartbeat).
+      key. Until confirmed, the shared key still works for this one worker — a
+      window that closes on the worker's first own-key heartbeat OR after
+      ``WORKER_KEY_CONFIRM_WINDOW`` from the key being minted, whichever comes
+      first.
+
+    That second bound is the whole point of this function. The window used to
+    close only on confirmation, so a worker that CANNOT persist its key — a
+    pre-1.0.0 image, a read-only /data, an ephemeral container — never closed it
+    at all. The security cutover the per-worker keys exist for silently never
+    completed: anyone holding CASHPILOT_API_KEY could impersonate that worker
+    indefinitely, and the UI re-sent the key to them every 60 seconds while
+    logging "not yet confirmed" once a minute forever.
     - ``"ok"``      — worker presented its own key: authenticated; confirm it so the
       shared key is refused from now on (the cutover finalizes).
 
@@ -3488,7 +3541,28 @@ async def _authenticate_worker_heartbeat(request: Request, cid: str) -> str:
     if token and hmac.compare_digest(token.encode(), key.encode()):
         return "ok"
     if not confirmed and shared_ok:
-        return "reissue"
+        if await _enrollment_window_open(cid):
+            return "reissue"
+        # Past the window. Refusing here is the cutover finally completing for a
+        # worker that could never complete it itself. Said plainly, because the
+        # operator's next question is why a worker that was working stopped.
+        logger.warning(
+            "Worker '%s' presented the SHARED key %s after enrolling and has never used its own key. "
+            "The shared key is no longer accepted for it: a worker that cannot persist "
+            "/data/.worker_key would otherwise keep the shared key valid for its identity forever. "
+            "Upgrade that worker to 1.0.0+, give it a writable and PERSISTENT /data, then remove it "
+            "in the fleet page so it can enroll again.",
+            cid,
+            _humanize_window(),
+        )
+        raise HTTPException(
+            status_code=401,
+            detail=(
+                "Enrollment was never completed for this worker and the shared key is no longer "
+                "accepted for it. Upgrade the worker to 1.0.0+ with a writable, persistent /data, "
+                "then remove it in the fleet page so it can enroll again."
+            ),
+        )
     raise HTTPException(status_code=401, detail="Invalid or missing per-worker key")
 
 
@@ -3565,6 +3639,11 @@ async def api_list_workers(request: Request) -> list[dict[str, Any]]:
         raw_watts = config.get(f"worker_{_worker_config_key(w)}_watts") or config.get(f"worker_{w.get('id')}_watts")
         w["watts"] = raw_watts or ""
         w["dedicated"] = _worker_flag(config, w, "dedicated")
+        # Whether this worker ever finished the per-worker-key cutover. Without
+        # it the only trace was a UI log line once a minute, which nobody reads,
+        # so a worker still authenticating with the SHARED key looked identical
+        # to a fully enrolled one on the fleet page.
+        w["enrollment"] = enrollment_state(w.get("key_issued_at"), bool(w.get("key_confirmed")))
     return workers
 
 

app/templates/fleet.html

@@ -310,6 +310,11 @@ <h2 class="section-title">Workers</h2>
                 <span style="font-weight: 600; color: var(--text-primary);">${esc(w.name)}</span>
                 ${isAndroid ? '<span style="font-size:0.7rem; padding:2px 6px; border-radius:4px; background:var(--bg-hover); color:var(--text-muted);">Android</span>' : ''}
                 ${nameCounts[String(w.name || '')] > 1 ? `<span style="font-size:0.7rem; padding:2px 6px; border-radius:4px; background:var(--warning-soft, var(--bg-hover)); color:var(--warning);" title="More than one worker is registered under this name. That usually means this host was redeployed with a new or empty /data, so it re-registered under a new id and the old row stayed behind. Compare the ids and the last-seen times before removing one.">duplicate name</span>` : ''}
+                ${w.enrollment === 'incomplete'
+                  ? `<span style="font-size:0.7rem; padding:2px 6px; border-radius:4px; background:var(--warning-soft, var(--bg-hover)); color:var(--warning);" title="This worker enrolled but never used its own key, so it is still authenticating with the shared CASHPILOT_API_KEY. The shared key is no longer accepted for it. It usually means the worker cannot persist /data/.worker_key — an old image, or a /data that is read-only or not a persistent volume. Upgrade it to 1.0.0+ with a writable persistent /data, then remove it here so it can enroll again.">enrollment incomplete</span>`
+                  : w.enrollment === 'pending'
+                  ? `<span style="font-size:0.7rem; padding:2px 6px; border-radius:4px; background:var(--bg-hover); color:var(--text-muted);" title="This worker has been issued its own key but has not used it yet, so the shared key still works for it for a short while. It clears itself on the worker's next heartbeat.">enrolling</span>`
+                  : ''}
                 <code style="font-size:0.68rem; color:var(--text-muted); background:var(--bg-hover); padding:1px 5px; border-radius:3px;" title="This worker's client_id — the id CashPilot keys it on. Two rows with the same name always have different ids; this is how to tell which is which.">${esc(w.client_id || ('#' + w.id))}</code>
               </div>
               ${_isOwner ? `<button class="btn btn-danger btn-sm btn-remove-worker" data-worker-id="${w.id}" data-worker-name="${esc(w.name)}" data-worker-client="${esc(w.client_id || '')}" title="Remove worker">Remove</button>` : ''}

docs/upgrade-v1.md

@@ -32,9 +32,17 @@ You don't handle keys by hand — this all happens on the next heartbeat.
 
 1. **Upgrade the UI** image to `drumsergio/cashpilot:1.0.0` (or newer).
 2. **Upgrade every worker** image to `drumsergio/cashpilot-worker:1.0.0` (or newer).
-   Do not leave old-version workers running against a v1.0.0 UI — once the UI has
-   enrolled a worker, an old worker image (which only knows the shared key) can no
-   longer heartbeat.
+   Do not leave old-version workers running against a v1.0.0 UI. An old worker
+   image only knows the shared key and cannot persist the one it is issued, so it
+   never finishes enrolling: for the first 24 hours it keeps heartbeating on the
+   shared key — which means anyone holding `CASHPILOT_API_KEY` can impersonate it
+   for that long — and after that it is refused and goes offline. The fleet page
+   marks such a worker **enrollment incomplete**.
+
+   The same thing happens to a current worker image whose `/data` is read-only or
+   is not a persistent volume, because it has nowhere to keep `/data/.worker_key`.
+   To recover one: fix the image or the volume, then remove the worker in the
+   fleet page so it enrolls again from scratch.
 3. Keep `CASHPILOT_API_KEY` **unchanged** — it is still needed for enrollment.
 4. Restart the containers. Each worker auto-enrolls on its first heartbeat; confirm
    every worker shows **online** in the fleet dashboard.