From d28494d21fd840e63fa8245df7e9819e304167cd Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Wed, 30 Sep 2026 15:17:40 +0200 Subject: [PATCH] Record the known framework limitations in a TODO Mode flapping under umask 002, broad NOPASSWD sudo, runner self-updates, per-secret vault reads and fact caching, each with its cost and fix. Co-Authored-By: Claude Opus 5.5 --- README.md | 1 + TODO.md | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 TODO.md diff --git a/README.md b/README.md index 18b339a..736cb5f 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ scripts/check.sh # the full static gate — what CI runs skills/adding-a-stack/ # step-by-step guide to adding a service docs/ # bootstrap, secrets, CI setup, connectivity, deploys AGENTS.md # the conventions and the role contract, in full +TODO.md # known defects and deferred decisions ``` ## Getting started diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..f615f3c --- /dev/null +++ b/TODO.md @@ -0,0 +1,88 @@ +# TODO + +Known defects and deferred decisions in the framework, each with what it costs +to leave alone and what fixing it would take. Nothing here blocks a deploy. +Items are listed worst first. + +## 1. The payload sync fights the role over `stack_dest`'s mode + +`roles/compose_stack/tasks/sync.yml` creates `{{ stack_dest }}` with an explicit +`mode: "0755"`, and then `ansible.posix.synchronize` runs with `-a`, which +propagates the *source* directory's permissions onto it. A control node with +`umask 002` produces a `0775` payload directory, so every run alternates: the +sync sets `0775`, the `file` task sets it back to `0755`, and both report a +change. + +**Cost.** Cosmetic: the mode ends up correct either way. But it makes rule 8 of +`AGENTS.md` false ("a second run must report zero changes") for anyone deploying +from a `umask 002` workstation, which is most Debian and Ubuntu desktops. A rule +that is known-false is worse than no rule. + +**Fix.** Either drop the explicit `mode: "0755"` from *Create the stack +directory* and let the sync own it, or stop the sync carrying permissions at all +with `--no-perms --omit-dir-times` in `rsync_opts`. Pinning them outright with +`--chmod=D755,F644` is the most explicit, but would strip the executable bit +from payload scripts such as `webapp`'s `app/migrate.sh`, so it needs an +exception for those. + +## 2. `NOPASSWD:ALL` is broader than the role needs + +`bootstrap.yml -e deploy_sudo=true` writes `deploy ALL=(ALL) NOPASSWD:ALL`. The +role escalates for exactly one thing: `ansible.builtin.file` setting `owner` or +`group` on a `stack_dirs` entry. Anyone who can dispatch a deploy — or who +obtains the deploy key — can do anything on the host. + +**Cost.** It widens what a compromised CI secret is worth. + +**Fix.** Scope the sudoers file to the commands `ansible.builtin.file` actually +invokes. Worth measuring against a real run before writing the rule, because a +too-narrow sudoers file fails at deploy time rather than at bootstrap time. + +## 3. A self-hosted runner cannot be updated from CI + +Not a stack in this repository, but the first thing most adopters add. A deploy +that recreates the runner kills the job doing the deploy, so a runner stack has +to stay out of `site.yml` and the deploy form and refuse to run under CI (see +the table in `skills/adding-a-stack/SKILL.md`). Updating it is then a +workstation job that nothing prompts anyone to run. + +**Approaches considered.** + +- **A deferred self-recreate.** Sync and pull inside the job, then start + `sleep 30; docker compose up -d` on the host with `async`/`poll: 0` and exit + green. Smallest change, but green proves only that the pull succeeded, not + that the new runner came up. +- **A second, maintenance runner (preferred).** The runners update each other: + a separate stack with its own label that nothing else targets, and a workflow + on that label that deploys the main runner. Each playbook's refusal becomes + conditional on an environment variable saying which runner it is on. Give the + runners a `shutdown_timeout` and a matching `stop_grace_period`, so in-flight + jobs finish before one is replaced. +- **A systemd path unit on the host** that CI triggers by touching a file. Puts + the update logic outside Ansible and needs root to set up. + +## 4. One vault request per secret, even when they share a path + +`roles/compose_stack/tasks/secrets.yml` issues one `uri` call per `stack_secrets` +entry. `webapp`'s two variables read two fields of the same KV path and cost two +round trips. + +**Cost.** Negligible — the read is over loopback on the host. + +**Probably won't fix.** The 1:1 mapping between entries and `results` is what +keeps the assert loops short and readable, and that legibility is load-bearing +in a file whose whole job is to fail safely without printing a value. Grouping +by path would mean carrying an index. Revisit only if a stack appears with +enough secrets to make the round trips visible. + +## 5. Fact caching would persist secret-bearing variables + +Nothing configures `fact_caching` today, so the registered results of the vault +reads live only in memory for the duration of a play, and `secrets.yml` clears +them once the `.env` is written. + +**Cost.** None, currently. + +**Watch for.** If fact caching is ever turned on, registered variables become +candidates for being written to disk on the control node. `no_log` does not help +with that. Check `secrets.yml` before enabling it.