From 674e408f092ca47fee30dd7363ac6a8b276bf21c Mon Sep 17 00:00:00 2001 From: Fredrik Johansson Date: Sun, 12 Jul 2026 21:07:08 +0200 Subject: [PATCH] Reject keep-executes-locally, switch to a .keep-vault marker file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes based on further review: - Explicitly considers and rejects letting keep's server execute the redeploy directly on announce, even though co-location removes the credential/network objection that sank the CI-SSHes-in alternative. The reason it's still wrong: a fully compromised keep server today can't do anything worse than leak ciphertext, since it never holds a decryption key. Giving it exec capability breaks that property — a routine web app bug becomes "triggers arbitrary redeploys," not "leaks encrypted blobs." Also notes the concrete cost: doing this from inside a container needs the Docker socket mounted in, which is broadly equivalent to root on the host. - Replaces the "vault key = deploy directory name" assumption with an explicit .keep-vault marker file per project directory. Repo name, deploy directory name, and vault key are three independent strings in practice (confirmed by a real example already in this fleet) — no naming convention should assume any two of them match. Bootstrap is now: choose a vault key, keep push it, drop a one-line .keep-vault file. CI's announce step uses the same deliberately hardcoded key, never derived from the repo name automatically. Co-Authored-By: Claude Sonnet 5 --- PROPOSAL-announce-and-agent.md | 85 +++++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 12 deletions(-) diff --git a/PROPOSAL-announce-and-agent.md b/PROPOSAL-announce-and-agent.md index f903f76..99f8c7c 100644 --- a/PROPOSAL-announce-and-agent.md +++ b/PROPOSAL-announce-and-agent.md @@ -98,6 +98,38 @@ happens today (a plain `docker compose up -d`, relying on that project's own `pull_policy: always`) for anything not yet bootstrapped, rather than either blocking deploys entirely or hard-failing on missing vaults. +### Considered and rejected: `keep` executes the redeploy itself + +Since `keep`'s own server will run on the same VPS as the deploy targets, +it's tempting to skip the polling indirection entirely and have `keep` +shell out to the deploy script directly when it receives an `announce` — +no new SSH credential, no new network-reachable pathway, since nothing +is reaching *into* the box that wasn't already running on it. + +Rejected anyway, because it trades away a property that's been the +throughline of `keep`'s whole design: **today, a fully compromised +`keep` server — not just a compromised client identity — can't do +anything worse than leak ciphertext and metadata**, because the server +never holds a decryption key. The moment `keep` can also execute a +command on `announce`, that stops being true — a bug in the HTTP layer +(not even a cryptographic break; an ordinary web app bug, an auth check +wrong on some edge case) becomes "attacker triggers arbitrary +redeploys," not "attacker sees encrypted blobs they can't read." That's +a new class of bad outcome, not a bigger version of an existing one, and +it's not worth trading for skipping a polling loop. + +There's also a concrete operational cost, not just a principle: actually +running `docker compose up -d` from inside `keep`'s own process would +need either the Docker socket mounted into `keep`'s container +(`/var/run/docker.sock` — widely treated as equivalent to handing that +container root on the host) or running `keep` outside a container +entirely, throwing away the isolation its own `Dockerfile` already +provides today. + +`keep` stays a pure relay regardless of where it's deployed. Something +*outside* `keep`'s own process boundary does the polling and the +executing — see §4. + ## Design ### 1. `announce` — a distinct verb from `push`/`pull` @@ -172,16 +204,23 @@ for project in "${PROJECTS[@]}"; do echo "" echo "▶ $project" - # Best-effort: does this machine have a working keep grant for - # "$project/production"? If not — not bootstrapped onto keep yet, or - # never will be (a static site with no secrets) — fall through to - # exactly what this script already did before keep existed. Every - # keep call here is allowed to fail; failure means "keep doesn't - # apply to this project," never "stop the deploy." - if keep pull "$project/production" --out "$dir/.env.new" 2>/dev/null; then - mv "$dir/.env.new" "$dir/.env" - else - rm -f "$dir/.env.new" + # A vault key is NOT assumed to match the deploy-directory name — repo + # names, deploy directory names, and vault keys are three independent + # strings in practice (a repo can be named one thing, its deploy + # directory another, entirely by history/accident). .keep-vault is an + # explicit, one-time-per-project marker: if it's missing, this project + # was never bootstrapped onto keep (or never will be — a static site + # with no secrets), and the loop falls through to exactly what it did + # before keep existed. Every keep call here is allowed to fail; + # failure means "keep doesn't apply to this project," never "stop the + # deploy." + if [ -f "$dir/.keep-vault" ]; then + vault="$(cat "$dir/.keep-vault")" + if keep pull "$vault" --out "$dir/.env.new" 2>/dev/null; then + mv "$dir/.env.new" "$dir/.env" + else + rm -f "$dir/.env.new" + fi fi if (cd "$dir" && docker compose up -d 2>&1); then @@ -193,8 +232,12 @@ for project in "${PROJECTS[@]}"; do done ``` -Vault key convention: `/production`, using the exact directory -name already in `PROJECTS` — no separate mapping table to keep in sync. +Bootstrapping a project onto `keep` becomes: choose a vault key (no +naming constraint — doesn't need to match the repo or the directory), +`keep push` it, grant whichever recipients need it, and +`echo "" > "$dir/.keep-vault"`. One file, human-readable, +`cat`-able to check what's configured, no second array in this script to +keep in sync by hand as directories get renamed or reorganized. Notice `announce`/`watch` aren't actually load-bearing in this version — whatever already triggers this script (cron, systemd timer, or still @@ -249,6 +292,17 @@ project hasn't been bootstrapped onto `keep` yet): run: keep announce myapp/production --tag ${{ steps.meta.outputs.short_sha }} ``` +The vault key here (`myapp/production`) is a **literal, hardcoded string +in this specific repo's own workflow file** — never derived from +`${{ github.repository }}` or any other automatic source. Same reasoning +as §4's `.keep-vault` file: the repo's own name, its deploy directory's +name, and its vault key are three independent strings that happen to +often look similar and are not guaranteed to match (see Context — a real +example already exists in this fleet). Whoever bootstraps a project onto +`keep` should paste the same deliberately-chosen vault key into both this +workflow step and that project's `.keep-vault` file, not rely on either +side inferring it from something else. + `continue-on-error: true` is load-bearing, not incidental — a project without a `keep` identity registered yet must still build and push its image successfully. This step failing should be silent/expected for any @@ -287,6 +341,13 @@ not-yet-bootstrapped project, not a red X on the workflow. ## Open questions +- **`.keep-vault` format** — a bare vault-key string is enough for + today's one-environment-per-directory reality. If a directory ever + needs to deploy more than one environment from the same + `docker-compose.yml` (unlikely given the current fleet shape, but not + impossible), a bare string stops being enough and it'd need to become + a small key=value file or similar. Not designing that ahead of an + actual need — ship the bare-string version. - **Fleet-wide `pull_policy: always` sweep** — a prerequisite this proposal depends on but doesn't itself fix (see Context). Should happen before or alongside this work, as its own small pass, not