fix(memory): let the family write to the vault again after a token expires

stacklets/memory/hooks/on_start_ready.py

@@ -4,12 +4,13 @@
 every restart. Best-effort — a failed pull never blocks startup.
 Readers fall back to whatever is already on disk, or to the seed.
 
-It also re-derives the vault's `origin` URL on every run. Both halves
-of that URL rot on their own schedule: the host part when the Mac takes
-a new DHCP lease, the embedded token when Forgejo expires it. Neither
-is refreshed by anything else, so a clone made months ago quietly stops
-working. Re-pointing it here costs one git config write and makes a
-restart the fix.
+It also repairs the vault's `origin` URL on every run. Both halves of
+that URL rot on their own schedule, and they need different cures. The
+host part (a LAN IP baked in at clone time) is re-derived from the
+current config, because the answer is knowable. The embedded token is
+not: nothing anywhere holds a newer one, so a token Forgejo rejects is
+replaced with a freshly issued one rather than rewritten. Between them,
+a clone made months ago starts working again after a restart.
 """
 
 from __future__ import annotations
@@ -28,6 +29,8 @@
     point_remote_at,
     pull_vault,
     purge_local_generated_memory_pages,
+    reissue_write_token,
+    remote_rejects_credentials,
     vault_path_for,
     vault_remote_url,
 )
@@ -40,11 +43,31 @@ def run(ctx):
     # Loopback, not the LAN address: this hook runs on the machine
     # Forgejo is published from. See `host_code_url`.
     code_url = host_code_url(ctx.env.get("CODE_URL", ""))
-    token = ctx.secret("MEMORY_BOT_TOKEN")
-    remote = (
-        authenticated_remote(vault_remote_url(code_url), BOT_USERNAME, token)
-        if code_url and token else ""
-    )
+    admin_user = ctx.env.get("ADMIN_USER", "")
+    admin_password = ctx.env.get("ADMIN_PASSWORD", "")
+
+    def remote_for(tok: str) -> str:
+        if not (code_url and tok):
+            return ""
+        return authenticated_remote(
+            vault_remote_url(code_url), BOT_USERNAME, tok,
+        )
+
+    remote = remote_for(ctx.secret("MEMORY_BOT_TOKEN"))
+
+    # The token is minted once at install and read forever after, so a
+    # token Forgejo has since rejected cannot be re-derived from
+    # anything — only replaced. Until it is, every host-side write
+    # (a todo tick, an ontology edit) fails 401 and a restart changes
+    # nothing, because re-pointing the remote writes the dead token
+    # back. Checked before the pull so the pull gets the good one.
+    if remote and remote_rejects_credentials(remote):
+        if fresh := reissue_write_token(code_url, admin_user, admin_password):
+            ctx.secret("MEMORY_BOT_TOKEN", fresh)
+            remote = remote_for(fresh)
+            ctx.step("Memory: Forgejo rejected the stored token; issued a new one")
+        else:
+            ctx.step("Memory: Forgejo rejected the stored token and it could not be replaced")
 
     # If the vault never got cloned (install hook ran before code
     # stacklet was reachable, for example), try once more here. This
@@ -69,8 +92,6 @@ def run(ctx):
     # Seamless B1 migration for existing installs. Those instances will
     # not rerun on_install_success, so create/clone brain here when
     # missing and purge legacy generated pages from the source repo.
-    admin_user = ctx.env.get("ADMIN_USER", "")
-    admin_password = ctx.env.get("ADMIN_PASSWORD", "")
     if not (code_url and admin_user and admin_password):
         ctx.step("Memory brain not cloned and admin credentials missing; skipping")
         return

stacklets/memory/lib.py

@@ -113,6 +113,10 @@
 BOT_USERNAME = "memory-bot"
 BOT_EMAIL = "memory-bot@local"
 TOKEN_NAME = "memory-bot"
+# Name of the admin-owned write token host-side writers use. Reissuing
+# under the same name replaces the old one in place, so a repair reuses
+# the slot instead of leaving a trail of dead tokens behind it.
+WRITE_TOKEN_NAME = "memory-install"
 TOKEN_SCOPES = [
     "write:repository", "read:repository",
     "read:user", "write:organization",
@@ -275,7 +279,13 @@ def point_remote_at(repo_path: Path, remote_url: str, *,
     embedded credential (a Forgejo token that expired). Both are held
     in git's config, where nothing ever refreshes them. So every start
     re-points the remote at the URL the current config says it should
-    be, and neither kind of rot survives a `stack up`.
+    be, and a moved host no longer survives a `stack up`.
+
+    Note what this does *not* fix. The credential written here is
+    whatever the secret store holds, so re-pointing a remote whose
+    token Forgejo has since rejected writes the same dead token back,
+    faithfully, forever. Curing that needs a new token, not a new URL:
+    see `reissue_write_token`.
     """
     repo_path = Path(repo_path)
     if not (repo_path / ".git").exists():
@@ -286,6 +296,50 @@ def point_remote_at(repo_path: Path, remote_url: str, *,
     return rc == 0
 
 
+def remote_rejects_credentials(remote: str, *, timeout: int = 15) -> bool:
+    """True when the remote answers but refuses the credentials we hold.
+
+    Deliberately narrow. An unreachable Forgejo is not a credential
+    problem and must not trigger a reissue: the token on file may be
+    perfectly good and the code stacklet merely still starting. Only
+    git's own "authentication failed" wording counts.
+    """
+    rc, _, err = _git(["git", "ls-remote", remote, "HEAD"], timeout=timeout)
+    return rc != 0 and is_auth_failure(err)
+
+
+def reissue_write_token(code_url: str, admin_user: str, admin_password: str,
+                        *, name: str = WRITE_TOKEN_NAME) -> str:
+    """Mint a fresh Forgejo write token to replace one that stopped working.
+
+    The token is the third thing in a remote URL that rots, and the only
+    one nothing re-derives: it is minted once during install and read
+    forever after. When Forgejo expires it — or the code stacklet is
+    rebuilt and the account it belonged to goes with it — every
+    host-side write starts failing 401, and no restart helps, because
+    re-pointing the remote writes the same dead token back.
+
+    Forgejo issues tokens only to the owning account (admin rights are
+    not enough), so this needs the admin's own password, which is what
+    the start hook already holds for the brain migration.
+
+    Returns "" when Forgejo is unreachable or refuses. A repair that
+    cannot happen is not a startup failure: readers keep serving what
+    is already on disk.
+    """
+    if not (code_url and admin_user and admin_password):
+        return ""
+    admin = ForgejoClient(
+        url=code_url, admin_user=admin_user, admin_password=admin_password,
+    )
+    try:
+        if not admin.ping():
+            return ""
+        return admin.issue_token(admin_user, admin_password, name, TOKEN_SCOPES)
+    except (ForgejoError, OSError):
+        return ""
+
+
 def pull_vault(vault_path: Path, *, timeout: int = 30) -> bool:
     """Fast-forward the vault from its remote. Best-effort.
 
@@ -634,7 +688,10 @@ def update_memory(config: dict, repo_path: str,
     """
     secrets = config.get("secrets", {}) if config else {}
     token = secrets.get("memory__MEMORY_BOT_TOKEN", "")
-    code_url = _code_url_from_config(config)
+    # Host plane: `{code_url}` renders the LAN address a phone would
+    # click, and this runs on the machine Forgejo is published from.
+    # Every write went to that address until it moved. See `host_code_url`.
+    code_url = host_code_url(_code_url_from_config(config))
     if not (token and code_url):
         return {"error": "Forgejo credentials missing — run `stack up memory` first"}
 
@@ -1568,7 +1625,7 @@ def install_memory_to_forgejo_admin(
 
     # Use admin token for file writes (admin has write:repository scope).
     admin_token = admin.issue_token(
-        admin_user, admin_password, "memory-install", TOKEN_SCOPES,
+        admin_user, admin_password, WRITE_TOKEN_NAME, TOKEN_SCOPES,
     )
     admin_token_client = ForgejoClient(url=code_url, token=admin_token)
     seeds = install_seeds(

tests/stacklets/test_memory_token_repair.py

@@ -0,0 +1,128 @@
+"""Keeping the vault's Forgejo credentials working, not merely current.
+
+A remote URL holds three things that rot, and only two of them can be
+re-derived. The host part is knowable: the current config says what it
+should be. The path is fixed. The token is neither -- it is minted once
+during install and read forever after, so when Forgejo expires it (or
+the code stacklet is rebuilt and the account goes with it) nothing
+anywhere holds a newer one.
+
+That distinction is the whole subject of this file. Re-pointing a
+remote whose token is dead writes the dead token back, faithfully,
+every restart, while every host-side write fails 401 and the operator
+sees a sync that "skipped". The cure is a new token, and the tests
+below pin both halves of getting there: telling a rejected credential
+apart from an unreachable host, and refusing to burn a good token when
+Forgejo is merely still starting up.
+
+The third test asserts where a write actually lands, because a write
+addressed to the LAN IP is the failure that started all of this and no
+amount of reading the code proves the rewrite happened.
+"""
+
+from __future__ import annotations
+
+import os
+import sys
+from pathlib import Path
+
+import pytest
+
+_REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "memory"))
+
+from lib import (  # noqa: E402
+    reissue_write_token,
+    remote_rejects_credentials,
+    update_memory,
+)
+
+
+@pytest.fixture
+def auth_failing_remote(tmp_path, monkeypatch) -> str:
+    """A git transport that always answers "your credentials are wrong".
+
+    A real `git-remote-<scheme>` helper on PATH, so git produces the
+    failure itself and the code under test reads the same stderr
+    Forgejo's expired-token response produces, through the same path.
+    """
+    bindir = tmp_path / "fake-git-transports"
+    bindir.mkdir()
+    helper = bindir / "git-remote-authfail"
+    helper.write_text(
+        "#!/bin/sh\n"
+        "echo \"fatal: Authentication failed for 'authfail://memory.git'\" >&2\n"
+        "exit 1\n",
+        encoding="utf-8",
+    )
+    helper.chmod(0o755)
+    monkeypatch.setenv("PATH", f"{bindir}{os.pathsep}{os.environ['PATH']}")
+    return "authfail://memory.git"
+
+
+class TestTellingTheTwoFailuresApart:
+    """Only one kind of failure is cured by issuing a new token."""
+
+    def test_a_rejected_credential_is_recognised_as_one(self, auth_failing_remote):
+        assert remote_rejects_credentials(auth_failing_remote)
+
+    def test_an_unreachable_remote_is_not_a_credential_problem(self, tmp_path):
+        """The distinction that keeps a restart from destroying a good token.
+
+        Forgejo is routinely unreachable for a few seconds while the
+        code stacklet starts. Treating that as "the token is bad" would
+        reissue on every cold boot and, worse, would do it on the one
+        occasion the reissue itself is most likely to fail.
+        """
+        assert not remote_rejects_credentials(str(tmp_path / "not-a-repo.git"))
+
+
+class TestReissuingIsBestEffort:
+    """A repair that cannot happen must not fail the whole start hook."""
+
+    def test_without_an_admin_password_no_token_is_issued(self):
+        """Forgejo issues tokens only to the owning account, so admin
+        rights alone are not enough -- the password is required."""
+        assert reissue_write_token("http://127.0.0.1:9", "stackadmin", "") == ""
+
+    def test_an_unreachable_forgejo_yields_no_token_rather_than_raising(self):
+        """Port 9 (discard) refuses instantly. The caller keeps serving
+        what is on disk; it does not take startup down with it."""
+        assert reissue_write_token("http://127.0.0.1:9", "stackadmin", "pw") == ""
+
+
+class TestWritesGoToTheHostsOwnAddress:
+
+    def test_a_write_reaches_loopback_when_config_names_the_lan_address(
+        self, httpserver,
+    ):
+        """`{code_url}` renders the address a phone on the couch clicks.
+
+        Baked into a host-side write that is a time bomb: the next DHCP
+        lease moves it and every todo tick starts failing. Here the
+        configured URL points at TEST-NET-1 (RFC 5737, guaranteed
+        unroutable) on the port the test server is really listening on,
+        so the request can only arrive if it was rewritten to loopback.
+        """
+        httpserver.expect_request("/api/v1/version").respond_with_json({})
+
+        result = update_memory(
+            {
+                "secrets": {
+                    "memory__MEMORY_BOT_TOKEN": "t0ken",
+                    "__code_url": f"http://192.0.2.1:{httpserver.port}",
+                },
+            },
+            "family/education/todos.md",
+            lambda text: text + "\n- [ ] something\n",
+            actor="homer",
+            message="chore(todos): homer added something",
+        )
+
+        assert httpserver.log, (
+            "the write never reached loopback, so it was addressed to the "
+            "LAN literal in the config"
+        )
+        # The transform's own outcome is beside the point here; what is
+        # pinned is the address it was sent to.
+        assert isinstance(result, dict)