diff --git a/docs/stack-reference.md b/docs/stack-reference.md index dcce2087..12c2b280 100644 --- a/docs/stack-reference.md +++ b/docs/stack-reference.md @@ -106,6 +106,7 @@ requires = ["core", "messages"] | `type` | string | `"docker"` | `"docker"` (default) or `"host"`. Host stacklets install native macOS software (brew, compiled binaries) alongside optional Docker containers. | | `requires` | list | `[]` | Stacklet IDs that must be enabled before this one. The runtime enforces ordering on `stack up` and prevents destroying dependencies. | | `build` | bool | false | If true, the stacklet has a local Dockerfile. `stack up` rebuilds the image on every run instead of pulling from a registry. Use for stacklets with custom code (bots, agents). | +| `required_secrets` | list | `[]` | Secret names (unprefixed, as `ctx.secret()` reads them) the stacklet cannot work without. `stack doctor` reports any that are absent and points at `stack setup `. Declare a secret here when it is minted by `on_install_success`, since that hook never runs again on an instance that is already installed. | ### Upstream diff --git a/lib/stack/cli.py b/lib/stack/cli.py index 587d13b0..4ed095da 100644 --- a/lib/stack/cli.py +++ b/lib/stack/cli.py @@ -838,13 +838,27 @@ def handle_doctor(stck, args): preferred = stck._cfg("core", "runtime", "orbstack") docker.init_runtime(preferred) - stacklets = sorted(s["id"] for s in stck.discover()) + discovered = stck.discover() + stacklets = sorted(s["id"] for s in discovered) + manifests = {s["id"]: s["manifest"] for s in discovered} + + def missing_secrets(stacklet_id): + """Declared credentials the secret store cannot produce. + + `required_secrets` names them the way a stacklet's own hooks do, + without the namespace prefix, so the manifest reads the same as + the `ctx.secret("MEMORY_BOT_TOKEN")` call that consumes it. + """ + required = manifests.get(stacklet_id, {}).get("required_secrets", []) + return [name for name in required if not stck.secret(stacklet_id, name)] + findings = doctor.diagnose( stacklets, stck.env, docker.containers_for, docker.container_env, docker.image_env, + missing_secrets=missing_secrets, ) print() diff --git a/lib/stack/doctor.py b/lib/stack/doctor.py index 94fd5257..4366f327 100644 --- a/lib/stack/doctor.py +++ b/lib/stack/doctor.py @@ -124,6 +124,40 @@ def check_exited(container: str, exit_code: int, since: str) -> Finding | None: ) +def check_missing_secrets(stacklet: str, missing: list[str]) -> Finding | None: + """A stacklet whose declared credentials were never provisioned. + + Secrets are minted by `on_install_success`, which runs once, on the + first install. A stacklet that grows a new credential later leaves + every existing instance without it: the hook has already run and + will not run again, so the gap is permanent and silent. `memory` + did exactly that, and the symptom reached the operator as a vault + write failing with "Forgejo credentials missing" on a stack whose + containers were all green. + + Generic on purpose, in the spirit of the rest of this module: the + stacklet says which keys it cannot work without (`required_secrets` + in its manifest) and the caller says which are absent. Nothing here + knows what a Forgejo token is, so the same rule covers whatever + credential the next stacklet adds. + + Names only, never values - see `env_drift`. + """ + if not missing: + return None + return Finding( + level=ERROR, + title=f"{stacklet} is missing credentials it needs", + detail=( + ", ".join(missing) + + " declared as required but absent from the secret store. " + "These are provisioned once, during install, so a stacklet " + "installed before it started needing one never gets it." + ), + fix=f"stack setup {stacklet}", + ) + + def check_endpoint(name: str, url: str, reachable: bool) -> Finding | None: """A configured endpoint that does not answer. @@ -142,12 +176,14 @@ def check_endpoint(name: str, url: str, reachable: bool) -> Finding | None: def diagnose(stacklets, rendered_env, containers_for, container_env, - image_env) -> list[Finding]: + image_env, *, missing_secrets=None) -> list[Finding]: """Run every check across the given stacklets. - The five collaborators are injected rather than imported so the whole - walk is testable with plain dicts - no Docker, no instance. Each is a + The collaborators are injected rather than imported so the whole walk + is testable with plain dicts - no Docker, no instance. Each is a callable taking a stacklet id (or container name) and returning facts. + `missing_secrets` is optional so a caller that has no secret store to + consult still gets the container checks. A stacklet whose env cannot be rendered is skipped rather than fatal: one misconfigured stacklet should not stop the others being diagnosed, @@ -157,8 +193,15 @@ def diagnose(stacklets, rendered_env, containers_for, container_env, for stacklet in stacklets: containers = containers_for(stacklet) if not containers: + # Nothing running means the stacklet is not part of this + # instance, so its missing credentials are not yet a problem. continue + if missing_secrets: + found = check_missing_secrets(stacklet, missing_secrets(stacklet)) + if found: + findings.append(found) + try: rendered = rendered_env(stacklet) except Exception: diff --git a/stacklets/docs/auth.py b/stacklets/docs/auth.py new file mode 100644 index 00000000..7ec8d4bb --- /dev/null +++ b/stacklets/docs/auth.py @@ -0,0 +1,86 @@ +"""Getting, and keeping, the Paperless API token. + +Every write famstack makes to Paperless carries this token. The +archivist files documents with it (`core` renders it into the bot +runner as `PAPERLESS_TOKEN`), and the start hook seeds person tags and +the category taxonomy with it. It is not a secret we invent: Paperless +issues it against the admin's own credentials, which is what makes a +lost one always recoverable and a stale one always detectable. + +That matters because the token used to be obtained during install and +never again. Paperless binds a token to its database, so a +`stack destroy docs` + `stack up docs` cycle invalidates it, and an +instance installed before this stacklet stored one has none at all. +Either way the install hook had already run for the last time: the +archivist quietly stopped being able to file anything, `stack up docs` +skipped its seeding without saying why, and the only cure was +re-running setup by hand. + +Both hooks come through here now, so every `stack up docs` re-checks +the token it is holding and asks for a new one when the answer is no. +""" + +from __future__ import annotations + +DEFAULT_URL = "http://localhost:42020" + + +def ensure_api_token(ctx) -> str: + """Return a token Paperless currently accepts, obtaining one if needed. + + Returns "" when no token could be had, which happens two ways: there + are no admin credentials to authenticate as, or Paperless did not + answer. Callers treat that as "do nothing this run" rather than as a + failure, because the next start tries again and a stack coming up + with Paperless still migrating its database is ordinary. + """ + url = ctx.env.get("PAPERLESS_URL", DEFAULT_URL) + + stored = ctx.secret("API_TOKEN") + if stored and _token_accepted(ctx, url, stored): + return stored + if stored: + ctx.step("Stored API token is invalid — obtaining a new one") + + username = ctx.env.get("ADMIN_USER", "") + password = ctx.secret("ADMIN_PASSWORD") + if not (username and password): + ctx.step("No admin credentials — skipping API token") + return "" + + ctx.step("Obtaining API token...") + try: + data = ctx.http_post( + f"{url}/api/token/", + f"username={username}&password={password}", + ) + except Exception as e: + ctx.step(f"Could not obtain API token: {e}") + return "" + + fresh = data.get("token", "") + if not fresh: + ctx.step("Unexpected response from Paperless token endpoint") + return "" + + ctx.secret("API_TOKEN", fresh) + ctx.step("API token saved") + return fresh + + +def _token_accepted(ctx, url: str, token: str) -> bool: + """True when Paperless still answers to this token. + + Deliberately cannot tell "rejected" from "unreachable", and does not + need to: the caller's response to both is to ask for a new token, + and that request fails too when Paperless is down. The run ends with + the stored token untouched either way. + """ + try: + ctx.http_get( + f"{url}/api/documents/", + headers={"Authorization": f"Token {token}"}, + ) + return True + except Exception: + return False diff --git a/stacklets/docs/hooks/on_install_success.py b/stacklets/docs/hooks/on_install_success.py index 395d06f4..e1e74f0a 100644 --- a/stacklets/docs/hooks/on_install_success.py +++ b/stacklets/docs/hooks/on_install_success.py @@ -7,67 +7,28 @@ Also seeded on every `stack up docs` via on_start_ready.py so they stay in sync with users.toml and taxonomy.yaml changes. +The token comes from `auth.ensure_api_token`, which both hooks share, +so an instance that loses one does not have to wait for a reinstall. """ import sys from pathlib import Path -# seed.py lives one level up from hooks/ +# seed.py and auth.py live one level up from hooks/ sys.path.insert(0, str(Path(__file__).parent.parent)) +from auth import ensure_api_token from seed import seed_person_tags, seed_taxonomy def run(ctx): - env = ctx.env - secret = ctx.secret - step = ctx.step - http_post = ctx.http_post - http_get = ctx.http_get - - PAPERLESS_URL = env.get("PAPERLESS_URL", "http://localhost:42020") - - # Verify existing token still works (a previous destroy + up cycle - # creates a fresh database, invalidating the old token in secrets.toml) - existing_token = secret("API_TOKEN") - token_valid = False - if existing_token: - try: - http_get( - f"{PAPERLESS_URL}/api/documents/", - headers={"Authorization": f"Token {existing_token}"}, - ) - token_valid = True - except Exception: - step("Stored API token is invalid — obtaining a new one") - - if not token_valid: - username = env.get("ADMIN_USER", "") - password = secret("ADMIN_PASSWORD") - if not username or not password: - step("No admin credentials — skipping API token") - return - - step("Obtaining API token...") - try: - data = http_post( - f"{PAPERLESS_URL}/api/token/", - f"username={username}&password={password}", - ) - existing_token = data.get("token") - if existing_token: - secret("API_TOKEN", existing_token) - step("API token saved") - else: - step("Unexpected response from Paperless token endpoint") - return - except Exception as e: - step(f"Could not obtain API token: {e}") - return + token = ensure_api_token(ctx) + if not token: + return # ── Create admin-role users as superusers ──────────────────────── - _create_admin_users(ctx, existing_token) + _create_admin_users(ctx, token) # ── Seed person tags + category taxonomy ─────────────────────────── - _seed_taxonomy(ctx, existing_token) + _seed_taxonomy(ctx, token) def _create_admin_users(ctx, token): diff --git a/stacklets/docs/hooks/on_start_ready.py b/stacklets/docs/hooks/on_start_ready.py index e75d30e3..3de21d24 100644 --- a/stacklets/docs/hooks/on_start_ready.py +++ b/stacklets/docs/hooks/on_start_ready.py @@ -4,17 +4,26 @@ person tags and category taxonomy stay in sync. Idempotent -- skips existing entries, creates new ones for users or categories added since last run. + +It also makes sure there is a working API token to seed with. This +hook used to read one and give up silently when it found none, which +is the state any instance predating the stored token was in: seeding +skipped every start, and the archivist -- which gets the same token +through rendered container env -- could not file a document. Paperless +will issue a replacement whenever asked, so there is nothing to give +up about. See auth.py. """ import sys from pathlib import Path sys.path.insert(0, str(Path(__file__).parent.parent)) +from auth import ensure_api_token from seed import seed_person_tags, seed_taxonomy def run(ctx): - token = ctx.secret("API_TOKEN") + token = ensure_api_token(ctx) if not token: return diff --git a/stacklets/memory/hooks/on_start_ready.py b/stacklets/memory/hooks/on_start_ready.py index da3c813b..e7ed1e94 100644 --- a/stacklets/memory/hooks/on_start_ready.py +++ b/stacklets/memory/hooks/on_start_ready.py @@ -8,9 +8,10 @@ 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. +not: nothing anywhere holds a newer one, so a token Forgejo rejects — +or one that was never stored at all — 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 @@ -53,7 +54,8 @@ def remote_for(tok: str) -> str: vault_remote_url(code_url), BOT_USERNAME, tok, ) - remote = remote_for(ctx.secret("MEMORY_BOT_TOKEN")) + token = ctx.secret("MEMORY_BOT_TOKEN") + remote = remote_for(token) # The token is minted once at install and read forever after, so a # token Forgejo has since rejected cannot be re-derived from @@ -61,13 +63,30 @@ def remote_for(tok: str) -> str: # (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): + # + # A token that was never stored needs the same cure and used to get + # none: instances installed before this hook's sibling learned to + # persist one hold nothing, a missing token builds no remote, and + # the repair below only ran once there was a remote to test. Those + # instances answered "Forgejo credentials missing" to every vault + # write until someone re-ran setup by hand. Both causes reduce to + # "we hold no credential Forgejo accepts", so both mint one. + if not code_url: + reason = "" + elif not token: + reason = "Memory: no vault write token on file" + elif remote_rejects_credentials(remote): + reason = "Memory: Forgejo rejected the stored token" + else: + reason = "" + + if reason: 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") + ctx.step(f"{reason}; issued a new one") else: - ctx.step("Memory: Forgejo rejected the stored token and it could not be replaced") + ctx.step(f"{reason} 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 diff --git a/stacklets/memory/stacklet.toml b/stacklets/memory/stacklet.toml index fd54b7d7..13ef2a42 100644 --- a/stacklets/memory/stacklet.toml +++ b/stacklets/memory/stacklet.toml @@ -29,6 +29,12 @@ requires = ["code"] # tweaks (`quartz/*.ts`) land without manual image management. build = true +# The Forgejo write token every host-side vault write needs. It is minted +# during install and stored as a secret; `stack doctor` reports it missing +# so an instance that predates the token (or lost it with a rebuilt code +# stacklet) says so plainly instead of failing one write at a time. +required_secrets = ["MEMORY_BOT_TOKEN"] + # LAN port for the wiki's Quartz preview server. Sits at the end of # the 42xxx range used by other stacklets so future infra ports stay # easy to scan. diff --git a/tests/framework/test_doctor.py b/tests/framework/test_doctor.py index b1741758..ca8cc302 100644 --- a/tests/framework/test_doctor.py +++ b/tests/framework/test_doctor.py @@ -12,6 +12,7 @@ check_endpoint, check_env_drift, check_exited, + check_missing_secrets, compose_supplied, diagnose, env_drift, @@ -174,6 +175,69 @@ def test_nonzero_exit_names_the_container_and_code(): assert "3 weeks ago" in finding.detail +# ── missing credentials ────────────────────────────────────────────────── +# +# The real incident: `memory` mints its Forgejo write token in +# on_install_success, which only ever runs on first install. Instances set +# up before that hook learned to persist the token held none, every vault +# write answered "Forgejo credentials missing", and doctor -- which knew +# only about containers -- reported a perfectly healthy stack. + +def test_a_declared_credential_that_is_absent_is_an_error(): + finding = check_missing_secrets("memory", ["MEMORY_BOT_TOKEN"]) + assert finding.is_error + assert "MEMORY_BOT_TOKEN" in finding.detail + assert finding.fix == "stack setup memory" + + +def test_all_credentials_present_is_not_a_finding(): + assert check_missing_secrets("memory", []) is None + + +def test_missing_credentials_are_reported_in_one_finding(): + # One line per stacklet, not per key: a stacklet whose provisioning + # never ran is missing all of them, and the cure is a single command. + finding = check_missing_secrets("docs", ["API_TOKEN", "BOT_PASSWORD"]) + assert "API_TOKEN" in finding.detail and "BOT_PASSWORD" in finding.detail + assert finding.fix == "stack setup docs" + + +def test_a_stacklet_that_declares_no_secrets_is_never_flagged(): + # The collaborator is optional; stacklets that declare nothing must + # not start reporting findings the moment the check ships. + containers = [{"name": "stack-x-1", "state": "running", + "exit_code": 0, "since": "Up 1 minute"}] + findings = diagnose( + ["x"], lambda s: {"A": "1"}, lambda s: containers, lambda n: {"A": "1"}, + lambda n: {}, missing_secrets=lambda s: [], + ) + assert findings == [] + + +def test_diagnose_finds_the_missing_vault_token(): + # End to end through the walk: this is the production symptom doctor + # was silent about. + containers = [{"name": "stack-memory-wiki", "state": "running", + "exit_code": 0, "since": "Up 2 days"}] + findings = diagnose( + ["memory"], lambda s: {}, lambda s: containers, lambda n: {}, + lambda n: {}, missing_secrets=lambda s: ["MEMORY_BOT_TOKEN"], + ) + assert len(findings) == 1 + assert findings[0].fix == "stack setup memory" + + +def test_a_stacklet_that_was_never_installed_is_not_flagged(): + # No containers means the stacklet is not part of this instance. Its + # credentials are supposed to be absent, and telling the reader to set + # up something they never asked for is noise. + findings = diagnose( + ["memory"], lambda s: {}, lambda s: [], lambda n: {}, lambda n: {}, + missing_secrets=lambda s: ["MEMORY_BOT_TOKEN"], + ) + assert findings == [] + + def test_reachable_endpoint_is_not_a_finding(): assert check_endpoint("AI", "http://localhost:42199/v1", reachable=True) is None diff --git a/tests/stacklets/test_docs_token.py b/tests/stacklets/test_docs_token.py new file mode 100644 index 00000000..22ef8e52 --- /dev/null +++ b/tests/stacklets/test_docs_token.py @@ -0,0 +1,156 @@ +"""The Paperless token, and the instances that lost it. + +The archivist's ability to file a document rests entirely on this +token: `core` renders it into the bot runner as `PAPERLESS_TOKEN`, and +the start hook seeds tags and taxonomy with it. It was obtained during +install and never again, which is fine until it isn't -- a +`stack destroy docs` cycle gives Paperless a new database and +invalidates it, and an instance installed before the stacklet stored +one has none at all. Both leave the install hook already spent, so no +restart could recover, and the failure showed up as documents that +never got filed rather than as an error. + +These tests are written against Paperless's actual token contract (a +`POST /api/token/` answering `{"token": ...}`, a bearer check that +401s) rather than against a recording of our own client, so they still +mean something if the implementation is rewritten. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "docs")) +sys.path.insert(0, str(_REPO_ROOT / "lib")) + +from stack.hooks import StackContext # noqa: E402 + +from auth import ensure_api_token # noqa: E402 isort:skip + + +class _Secrets: + """The slice of the secret store `ctx.secret` drives, with its + stacklet-then-global fallback intact -- ADMIN_PASSWORD is a global + secret and the token is a docs-scoped one, so a store that ignored + the distinction would let a broken lookup pass.""" + + def __init__(self, values: dict): + self.values = dict(values) + + def get(self, stacklet_id, name): + return (self.values.get(f"{stacklet_id}__{name}") + or self.values.get(f"global__{name}")) + + def set(self, stacklet_id, name, value): + self.values[f"{stacklet_id}__{name}"] = value + + +def _ctx(url: str, secrets: dict, admin_user: str = "stackadmin"): + """A real StackContext over a fake secret store. + + The context's own HTTP helpers are the ones under test as much as + anything else -- they are what turns a 401 into the exception the + token check reads -- so they are left alone and pointed at a real + server. + """ + steps: list[str] = [] + ctx = StackContext( + stack=SimpleNamespace(secrets=_Secrets(secrets)), + stacklet_id="docs", + env={"PAPERLESS_URL": url, "ADMIN_USER": admin_user}, + step_fn=steps.append, + ) + ctx.steps = steps # for assertions on what the operator saw + return ctx + + +@pytest.fixture +def paperless(httpserver): + """A Paperless that issues `fresh-t0ken` and honours only that token.""" + httpserver.expect_request( + "/api/token/", method="POST", + ).respond_with_json({"token": "fresh-t0ken"}) + httpserver.expect_request( + "/api/documents/", method="GET", + headers={"Authorization": "Token fresh-t0ken"}, + ).respond_with_json({"count": 0}) + return httpserver.url_for("").rstrip("/") + + +class TestAnInstanceWithNoToken: + """The production shape: installed before the token was ever stored.""" + + def test_a_token_is_obtained_and_saved(self, paperless): + ctx = _ctx(paperless, {"global__ADMIN_PASSWORD": "hunter2"}) + + assert ensure_api_token(ctx) == "fresh-t0ken" + assert ctx.secret("API_TOKEN") == "fresh-t0ken", ( + "the token was used for this run but never persisted, so the " + "archivist -- which reads it from the secret store -- stays blind" + ) + + def test_nothing_is_attempted_without_admin_credentials(self, paperless): + """Paperless issues tokens against a real login, so with no admin + password there is nothing to ask with. Saying so beats a stack + trace from a request that could never have worked.""" + ctx = _ctx(paperless, {}) + + assert ensure_api_token(ctx) == "" + assert any("admin credentials" in s for s in ctx.steps) + + +class TestATokenThatStoppedWorking: + """A destroy + up cycle hands Paperless a new database, and every + token minted against the old one is now a 401.""" + + def test_a_rejected_token_is_replaced(self, paperless): + ctx = _ctx(paperless, { + "docs__API_TOKEN": "from-the-old-database", + "global__ADMIN_PASSWORD": "hunter2", + }) + + assert ensure_api_token(ctx) == "fresh-t0ken" + assert ctx.secret("API_TOKEN") == "fresh-t0ken" + + def test_a_working_token_is_reused(self, paperless): + """Not merely an optimisation. The token reaches the archivist + through rendered container env, so churning it on every start + would leave a running bot holding one Paperless has moved on + from until something recreated the container.""" + ctx = _ctx(paperless, { + "docs__API_TOKEN": "fresh-t0ken", + "global__ADMIN_PASSWORD": "hunter2", + }) + + assert ensure_api_token(ctx) == "fresh-t0ken" + assert not any("Obtaining" in s for s in ctx.steps) + + +class TestPaperlessNotAnswering: + + def test_no_token_is_invented_when_the_service_is_down(self): + """Port 9 (discard) refuses instantly. A stack coming up with + Paperless still migrating is ordinary, so this must return + empty-handed rather than raise and take the hook down.""" + ctx = _ctx("http://127.0.0.1:9", {"global__ADMIN_PASSWORD": "hunter2"}) + + assert ensure_api_token(ctx) == "" + + def test_a_stored_token_survives_an_outage(self): + """The one case worth being careful about: an unreachable + Paperless looks exactly like a rejected token from here. Failing + to reach it must not clear what is on file, or a restart during + an outage would cost the instance a credential it still had.""" + ctx = _ctx("http://127.0.0.1:9", { + "docs__API_TOKEN": "still-good", + "global__ADMIN_PASSWORD": "hunter2", + }) + + ensure_api_token(ctx) + + assert ctx.secret("API_TOKEN") == "still-good" diff --git a/tests/stacklets/test_memory_token_repair.py b/tests/stacklets/test_memory_token_repair.py index 923b7fbc..e245ae1d 100644 --- a/tests/stacklets/test_memory_token_repair.py +++ b/tests/stacklets/test_memory_token_repair.py @@ -15,6 +15,14 @@ apart from an unreachable host, and refusing to burn a good token when Forgejo is merely still starting up. +A token that was never stored is the same hole seen from the other +side, and it is the one production hit: an instance installed before +the install hook learned to persist a token has none, `stack up memory` +had nothing to test and so repaired nothing, and every vault write +failed with "Forgejo credentials missing" until someone re-ran setup by +hand. The start hook is driven end to end below for that case, because +the bug lived in the branch that decides whether to repair at all. + 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. @@ -25,12 +33,15 @@ import os import sys from pathlib import Path +from types import SimpleNamespace import pytest _REPO_ROOT = Path(__file__).resolve().parent.parent.parent sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "memory")) +sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "memory" / "hooks")) +import on_start_ready # noqa: E402 from lib import ( # noqa: E402 reissue_write_token, remote_rejects_credentials, @@ -91,6 +102,132 @@ def test_an_unreachable_forgejo_yields_no_token_rather_than_raising(self): assert reissue_write_token("http://127.0.0.1:9", "stackadmin", "pw") == "" +class _RecordingCtx: + """The slice of hook context `on_start_ready` actually reads. + + A real `StackContext` wants an instance on disk, a parsed config and + a Docker runtime. The hook wants four things: the data dir, the + rendered env, the secret store, and somewhere to log progress. + Standing up only those four is what lets the hook be driven end to + end here, which matters because the bug this file guards lived in + `run()` itself and not in any helper it calls. + """ + + def __init__(self, data_dir: Path, env: dict, secrets: dict): + self.stack = SimpleNamespace(data=data_dir) + self.env = env + self.secrets = secrets + self.steps: list[str] = [] + + def secret(self, name, value=None): + if value is not None: + self.secrets[name] = value + return value + return self.secrets.get(name) # None when unset, as the store does + + def step(self, message): + self.steps.append(message) + + +@pytest.fixture +def forgejo_issuing_tokens(httpserver) -> str: + """A Forgejo that is up and will mint `fresh-t0ken` on request. + + Only the three calls `reissue_write_token` makes are served. Every + other path 500s, which is deliberate: the clone that follows the + repair then fails and the hook returns early, so these tests stay + about the credential decision and nothing downstream of it. + """ + httpserver.expect_request("/api/v1/version").respond_with_json({}) + httpserver.expect_request( + "/api/v1/users/stackadmin/tokens", method="GET", + ).respond_with_json([]) + httpserver.expect_request( + "/api/v1/users/stackadmin/tokens", method="POST", + ).respond_with_json({"sha1": "fresh-t0ken"}) + return httpserver.url_for("").rstrip("/") + + +class TestAnInstanceThatNeverStoredAToken: + """The production failure: no token at all, and no way back. + + Installs predating the persisting install hook hold no + `MEMORY_BOT_TOKEN`. Every host-side write answered "Forgejo + credentials missing" and every restart repaired nothing, because + the repair only ran once there was a remote to test and a missing + token builds no remote. The cure for "never had one" and for + "Forgejo rejected it" is the same call; only the trigger was wrong. + """ + + def _ctx(self, tmp_path, code_url, secrets): + return _RecordingCtx( + tmp_path / "data", + { + "CODE_URL": code_url, + "ADMIN_USER": "stackadmin", + "ADMIN_PASSWORD": "hunter2", + }, + secrets, + ) + + def test_a_missing_token_is_minted_on_the_next_start( + self, tmp_path, forgejo_issuing_tokens, + ): + ctx = self._ctx(tmp_path, forgejo_issuing_tokens, {}) + + on_start_ready.run(ctx) + + assert ctx.secrets.get("MEMORY_BOT_TOKEN") == "fresh-t0ken", ( + "an instance with no stored token stayed unable to write to its " + "own vault, which is the state production was found in" + ) + + def test_the_operator_is_told_a_credential_was_created( + self, tmp_path, forgejo_issuing_tokens, + ): + """A credential appearing out of nowhere is worth one line. + + Someone reading `stack up memory` should be able to connect a + vault that started working to the run that fixed it. + """ + ctx = self._ctx(tmp_path, forgejo_issuing_tokens, {}) + + on_start_ready.run(ctx) + + assert any("token" in step for step in ctx.steps) + + def test_a_token_already_on_file_is_left_alone( + self, tmp_path, forgejo_issuing_tokens, + ): + """Reissuing is a repair, not a routine. + + Forgejo deletes the old token when it issues a same-named + replacement, so minting on every start would invalidate the + credential any concurrent writer is holding. Here the remote + never answers "authentication failed" -- it is merely + unreachable for git -- so the stored token stands. + """ + ctx = self._ctx(tmp_path, forgejo_issuing_tokens, + {"MEMORY_BOT_TOKEN": "already-good"}) + + on_start_ready.run(ctx) + + assert ctx.secrets["MEMORY_BOT_TOKEN"] == "already-good" + + def test_no_token_is_invented_when_forgejo_is_down(self, tmp_path): + """Port 9 (discard) refuses instantly. + + The hook must still fall through to its existing "credentials + missing, skipping" path rather than failing the start. A stack + coming up with the code stacklet not yet listening is normal. + """ + ctx = self._ctx(tmp_path, "http://127.0.0.1:9", {}) + + on_start_ready.run(ctx) + + assert not ctx.secrets.get("MEMORY_BOT_TOKEN") + + class TestWritesGoToTheHostsOwnAddress: def test_a_write_reaches_loopback_when_config_names_the_lan_address(