From 0fdc11f10ec72d3b4dc047c8e2c63198b31d3667 Mon Sep 17 00:00:00 2001 From: Arthur Date: Sun, 2 Aug 2026 12:38:57 +0200 Subject: [PATCH] fix(memory): let the family write to the vault again after a token expires Ticking off a todo from chat failed with a 401 and no restart helped. Two causes, both invisible from the outside. The write seam still addressed Forgejo by the LAN address `{code_url}` renders for phones, so every host-side write went to whatever IP the Mac had when it was installed. Writes now go to loopback like the rest of the host plane. The token was worse: minted once during install and only ever read afterwards, so when Forgejo expired it nothing held a newer one, and re-pointing the remote rewrote the dead token every start. A token Forgejo rejects is now replaced with a freshly issued one. Only a rejected credential triggers that -- an unreachable Forgejo is left alone, so a slow cold start cannot burn a good token. --- stacklets/memory/hooks/on_start_ready.py | 47 +++++-- stacklets/memory/lib.py | 63 +++++++++- tests/stacklets/test_memory_token_repair.py | 128 ++++++++++++++++++++ 3 files changed, 222 insertions(+), 16 deletions(-) create mode 100644 tests/stacklets/test_memory_token_repair.py diff --git a/stacklets/memory/hooks/on_start_ready.py b/stacklets/memory/hooks/on_start_ready.py index 1690a92..da3c813 100644 --- a/stacklets/memory/hooks/on_start_ready.py +++ b/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 diff --git a/stacklets/memory/lib.py b/stacklets/memory/lib.py index c600a74..2026ba8 100644 --- a/stacklets/memory/lib.py +++ b/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( diff --git a/tests/stacklets/test_memory_token_repair.py b/tests/stacklets/test_memory_token_repair.py new file mode 100644 index 0000000..923b7fb --- /dev/null +++ b/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-` 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)