diff --git a/11-Features.md b/11-Features.md index 824a1ff..6fbe912 100644 --- a/11-Features.md +++ b/11-Features.md @@ -43,6 +43,7 @@ six weeks, and the table alone will not carry it. | [Isolated worktree per worker](#isolated-worktree-per-worker) | `fleet_spawn{worktree, ticket}` | CB-301-ext | `session/GitWorktrees` | | [Worker tool-surface isolation](#worker-tool-surface-isolation) | automatic | CB-525 | `session/GitWorktrees` | | [Worktree-hostile config isolation](#worktree-hostile-config-isolation) | automatic | CB-543 | `session/GitWorktrees` | +| [No credentialed remote URL reaches a worktree](#no-credentialed-remote-url-reaches-a-worktree) | automatic | CB-157 / CB-189 | `session/GitWorktrees` | | [Worker opens its own PR](#worker-opens-its-own-pr) | `gitTokenEnv:` / `gitHostEnv:` | CB-302 | `worker/HerdrPeerLauncher` | | [Session lifecycle caps](#session-lifecycle-caps) | `lifecycle:` | CB-303 | `session/SessionManager` | | [Keep a worktree that still holds work](#keep-a-worktree-that-still-holds-work) | automatic | CB-576 | `session/GitWorktrees` | @@ -309,6 +310,59 @@ became a quiet privilege leak, which makes this isolation load-bearing rather th server map for `.mcp.json`, and an empty `.autoenv`. A deletion could be undone by a later checkout; the skip-worktree bit keeps the safe local replacement from looking like work for a worker to commit. +## No credentialed remote URL reaches a worktree + +**What.** Before provisioning a member's worktree, fleetd makes sure the repository's remote URLs +carry no embedded credentials, and says so when they do. + +Three things happen, in this order: + +1. **Report.** Every remote is enumerated, and both its fetch and its push URL are checked for + user-info on any non-SSH scheme. Anything found is logged as a WARN naming the remote. +2. **Strip.** User-info is removed from `origin`'s HTTPS URL. +3. **Refuse.** If the provisioned worktree still resolves a credentialed HTTPS `origin`, the + provision fails rather than handing the member a URL with a secret in it. + +``` +WARN member worktree shares a remote URL containing user-info: remote=upstream + repository=/Users/…/fleetd; remove credentials from the repository's git config +``` + +**On.** Always on. There is no key. + +**Why.** A linked worktree **shares its parent repository's git config**. A token embedded in a +remote URL is therefore readable by the member the moment its worktree exists — and `git remote -v` +prints it in full, so it leaks again the first time anyone surveys the host. Members are meant to +push with the repo-scoped `WORKER_GITEA_TOKEN` injected at spawn, never with a credential baked into +config. + +**Gotcha — the report and the refusal cover different ground, on purpose.** Only `origin`, and only +HTTPS, is stripped and refused. Every *other* remote, every `pushurl`, and every other non-SSH scheme +is **reported and then left alone**. Rewriting a remote nobody asked us to touch is not fleetd's +call; telling you it is there is. So a WARN here is work for you to do, not something the daemon has +already handled. + +SSH URLs are deliberately exempt. There the user part selects an account and authentication happens +over the SSH transport, so `git@host` is not a credential the way `user:token@host` is. + +**The reason it warns before it strips.** For the `origin` HTTPS case you will see the WARN +immediately followed by the strip's own INFO line. That ordering is intentional: it leaves an audit +trail that there *was* something to fix. Reporting after the strip would silence the one case the +daemon actually repairs. + +**Gotcha — a reporting check must never break a provision.** An earlier attempt at this ran its git +calls unguarded at the top of `add()`, where any non-zero exit or the 30-second timeout would have +aborted the whole worktree provision. Every call here is wrapped, and a failure logs only the +exception's *class*, never its message — the message is read from the very config that may hold the +URL being looked for. + +For the same reason, every git command whose **stdout is a URL** runs through a redacting variant +that keeps captured output out of exception messages. Without it a failing `git remote get-url` +copies the credentialed URL into the exception, and from there into the log — defeating the check by +way of its own error path. + +--- + ## Worker opens its own PR **What.** Injects a repo-scoped forge token so a worker can commit, push over SSH, and open its own @@ -1678,21 +1732,42 @@ memberCredentials: 34 known name(s), 5 allowed — blocking 29 on every spawn **It also reports what you forgot.** On every spawn the launcher scans the host environment for names *shaped* like credentials (`TOKEN`, `SECRET`, `_KEY`, `APIKEY`, `PASSWORD`, `CREDENTIAL`, -`AUTH`) and warns about any that are on neither list. This is the part that pays for itself: the -first spawn after it deployed named two variables no hand-written list had ever contained, because -neither lives in the secret store — +`AUTH`) and reports any that are on neither list. This is the part that pays for itself: the first +spawn after it deployed named two variables no hand-written list had ever contained, because neither +lives in the secret store — ``` WARN memberCredentials gap: 2 credential-shaped env var name(s) are on neither known: nor allow: — every member pane inherits them UNBLOCKED — [CLAUDE_CODE_MESSAGING_TOKEN, SSH_AUTH_SOCK] ``` -`SSH_AUTH_SOCK` is the interesting one, and it is **allowed on purpose**: a worktree's remote is -`ssh://git@git.ltms.dev`, so without the agent socket a member cannot push at all. But that socket -lets a member sign with every key the operator's agent holds — strictly more power than the -repo-scoped forge token CB-302 built to avoid exactly this. Tracked as **CB-607**; the fix is to push -over HTTPS with `WORKER_GITEA_TOKEN` and then block it. Do not block it first, or members stop -pushing. +**Since 2026-08-31 the report distinguishes three cases, because one wording was lying.** A name +being on neither list does not by itself mean a member inherits it — under `allow-list` on a zsh +login shell the generated scrub blanks it anyway. The severity now follows what the scrub actually +does, decided with the same predicate the scrub itself evaluates: + +| Situation | Line | Meaning | +|---|---|---| +| `deny-by-default`, or `allow-list` on a non-zsh shell | **WARN** … `inherits them UNBLOCKED` | Nothing scrubs these. Real exposure. | +| `allow-list` + zsh, name **not** on the derived allow-list | **INFO** … `the scrub blanks them anyway` | Contained. Listing it just makes that explicit. | +| `allow-list` + zsh, name **kept** by the derived allow-list | **WARN** … `the derived allow-list keeps them anyway` | Real exposure, and easy to miss. | + +That last row is the one worth knowing about. The derived allow-list is a **superset** of +`known` + `allow`: it also picks up every profile's `gitTokenEnv`, `gitHostEnv`, `tokenEnv` and +`env:` keys. So naming a variable in a profile silently grants it to members, whether or not it is on +either list — and until this change the daemon reported that case as *contained*. Each report kind +has its own once-per-daemon guard, so a benign INFO can no longer suppress a serious WARN. + +**`SSH_AUTH_SOCK` is now blocked, and that is a downgrade, not a win.** It was once allowed on +purpose, because a worktree's remote was `ssh://git@git.ltms.dev` and a member could not push +without the agent socket. Members now push over HTTPS with `WORKER_GITEA_TOKEN`, so the live config +sets `sshAuthSock: block`, and `MemberEnvAllowList.derive` drops the name even if an operator lists +it under `allow:` — the config cannot re-grant it by accident. + +Do not read that as the problem being solved. #184 measured a member with the socket blanked pushing +**fine anyway**: the forge key is a readable, passphrase-free *file*, and `ssh -G` finds it outside +`~/.ssh`. An environment control cannot remove a file. Blocking the socket closes one door in a room +with another door open; the actual fix is a separate OS user (#185). **Gotcha — under `deny-by-default` the config half alone does not hold.** The launcher writes the member's environment at spawn, and then the member's pane runs a **login shell**, which re-sources @@ -2194,11 +2269,21 @@ filesystem: blocking `SSH_AUTH_SOCK` does not stop it, and no environment scrub scrub removes variables and the key is a file (#184). A separate OS user is the control; this key is the fleetd half of it. Design and measurements are in #185. -**Gotcha — the feature is not ready to switch on yet.** Four things must land first, and #185 keeps -the running list. The two that bite hardest: herdr creates its API socket mode `0600` and `umask` -does not change it, so cross-user access still needs a post-start `chmod g+rw` (a race on every -start); and pane ownership lives only in memory, so after a restart every surviving pane is unowned -and `fleet_stop` on it refuses rather than guess which daemon owns it. +**Gotcha — the feature is still not ready to switch on.** #185 keeps the running list. Two of the +blockers were cleared on 2026-08-31; the rest stand. + +*Cleared.* Pane ownership lives only in memory, so a restart used to leave every surviving pane +unowned and `fleet_stop` refused it. `CompositePeerLauncher` now **probes**: it asks each daemon +which one knows the pane, routes to the single match and caches it, treats no match as already +stopped, and refuses only a genuine two-daemon collision — with a message that says that is what +happened. One unreachable daemon no longer breaks the probe for panes owned by a healthy one. + +*Still open.* herdr creates its API socket mode `0600` and `umask` does not change it, so cross-user +access still needs a post-start `chmod g+rw` — a race on every start, and an upstream ask that has +not been made. Pane ids are still **not** daemon-qualified: `fleet_stop{paneId}` takes a bare +`w1:p1`, so the probe makes a collision fail loudly instead of silently closing the wrong pane, which +is a safety net, not the design fix. `hostEnvNames` still reads fleetd's own environment, which is +the wrong environment under a second user. Worktrees are still created as the operator's uid. **The shape to watch for.** herdr's workspace, tab and pane ids are **per-daemon sequential counters**. Two daemons really do both hold `w1:p1`, pointing at different panes owned by different @@ -2207,11 +2292,29 @@ every one of them passed the whole test suite, because with the key unset both c object. When reviewing anything on this path, ask each herdr call one question: *which daemon does this go to, and is that the daemon that owns the thing it is asking about?* -**Observable change even with one daemon.** `GET /healthz` now requires **both** daemons to answer +**Observable change even with one daemon.** `GET /healthz` requires **both** daemons to answer before it returns 200 (with one configured that is the single ping it always was). `GET /sessions` -merges `workspace.list` across both. The 200 body still reports only the lead daemon's herdr version -and protocol — a known gap, on the #185 list, and it matters because in two-daemon mode it is the -*member* daemon's protocol that decides whether spawns work. +merges `workspace.list` across both. + +Since 2026-08-31 the 200 body also reports the **member** daemon's version and protocol, under its +own `member` key, and adds `protocolMismatch: true` when the two protocol numbers differ: + +```json +{ "status": "ok", + "herdr": { "version": "0.8.0", "protocol": 3 }, + "member": { "version": "0.7.1", "protocol": 2 }, + "protocolMismatch": true } +``` + +The `member` key and `protocolMismatch` appear **only** when a second daemon is configured, so a +single-daemon body is byte-identical to before. `herdr` still carries the **lead** daemon's values, +deliberately: `scripts/redeploy-fleetd.sh` and `scripts/rename-checkout.sh` both read this endpoint, +and folding the two daemons into one key would hide a mismatch from whichever reader looks at only +that key. + +This closes a real trap. The member daemon was pinged and its answer thrown away, so a member herdr +running a mismatched protocol left `/healthz` green while **every spawn failed** — and it is the +member daemon's protocol, not the lead's, that decides whether a spawn works. ---