fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

CB-634: the parity overlay copies .env/.envrc into worker worktrees — the same shape as CB-633 #148

New Issue
Closed
opened 2026-08-23 07:53:56 +02:00 by ltms · 5 comments
ltms commented 2026-08-23 07:53:56 +02:00
Owner
Copy Link
Copy Source

Found while implementing #144 (CB-633), reported by the implementer and not fixed there — it was outside that unit's scope.

The shape

CB-633 exists because a control that lives in a file fleetd does not own can be undone by that file. The credential guard sat inside ${SHARED_ENV}/tools/secrets.sh and a later source handed the values straight back.

The CB-511 parity overlay has the same shape pointing the other way. Profile.parityOverlay defaults to copying .env and .envrc from the primary's checkout into every worker worktree. So a member's environment is shaped by files fleetd does not own and does not read — and a repo-committed .envrc can carry whatever it likes into a member.

direnv makes this concrete: an .envrc is executable shell that runs on cd. If the checkout has one, it runs inside the member with whatever it exports.

Why it is not simply "the same bug"

Two differences worth keeping straight, because they change the fix:

  • The overlay is deliberate. Parity is the feature: a worker whose worktree lacks the primary's local config cannot reproduce the primary's build. Removing the copy is not obviously right.
  • It is config-driven already (parityOverlay), so an operator can narrow it today. The defect is the default, not the absence of a knob.

But CB-633's allow-list runs after the login shell, and .envrc is evaluated by direnv on cd — which happens after the shell's startup files. So the scrub does not cover it. A credential re-exported by a copied .envrc survives CB-633 completely.

What to decide

  1. Does the allow-list scrub need to apply to direnv too, or is that out of scope? A direnv hook runs on every cd, so a one-shot scrub in .zlogin cannot catch it.
  2. Should .envrc come out of the default parityOverlay list, leaving .env (data, not executable)? That is the smaller change and it removes the executable half.
  3. Whatever is chosen, fleetd should log which overlay files it actually copied for each spawn. Right now the copy is silent, so nobody can tell from the logs whether a member got an .envrc at all — and a silent copy is what makes this hard to notice.

Acceptance

  • A decision recorded on this ticket for points 1 and 2, with the reason, before any code.
  • Point 3 regardless: name the copied files per spawn in the log. Report the denominator — copied N of M candidates, not copied N (#113).
  • A test that a member worktree gets exactly the configured overlay set and nothing else.

Not verified

I have not checked whether this repo's checkout actually has an .envrc, nor whether direnv is installed on this host. So this is a real defect in the mechanism, not a demonstrated live exposure. Establish that first — the answer changes the priority.

Found while implementing #144 (CB-633), reported by the implementer and not fixed there — it was outside that unit's scope. ## The shape CB-633 exists because **a control that lives in a file `fleetd` does not own can be undone by that file.** The credential guard sat inside `${SHARED_ENV}/tools/secrets.sh` and a later `source` handed the values straight back. The CB-511 parity overlay has the same shape pointing the other way. `Profile.parityOverlay` defaults to copying `.env` and `.envrc` from the primary's checkout into every worker worktree. So a member's environment is shaped by files `fleetd` does not own and does not read — and a repo-committed `.envrc` can carry whatever it likes into a member. `direnv` makes this concrete: an `.envrc` is executable shell that runs on `cd`. If the checkout has one, it runs inside the member with whatever it exports. ## Why it is not simply "the same bug" Two differences worth keeping straight, because they change the fix: - The overlay is **deliberate**. Parity is the feature: a worker whose worktree lacks the primary's local config cannot reproduce the primary's build. Removing the copy is not obviously right. - It is **config-driven** already (`parityOverlay`), so an operator can narrow it today. The defect is the **default**, not the absence of a knob. But CB-633's allow-list runs *after* the login shell, and `.envrc` is evaluated by `direnv` on `cd` — which happens **after** the shell's startup files. So the scrub does not cover it. A credential re-exported by a copied `.envrc` survives CB-633 completely. ## What to decide 1. Does the allow-list scrub need to apply to `direnv` too, or is that out of scope? A `direnv` hook runs on every `cd`, so a one-shot scrub in `.zlogin` cannot catch it. 2. Should `.envrc` come out of the default `parityOverlay` list, leaving `.env` (data, not executable)? That is the smaller change and it removes the executable half. 3. Whatever is chosen, `fleetd` should **log which overlay files it actually copied** for each spawn. Right now the copy is silent, so nobody can tell from the logs whether a member got an `.envrc` at all — and a silent copy is what makes this hard to notice. ## Acceptance - A decision recorded on this ticket for points 1 and 2, with the reason, before any code. - Point 3 regardless: name the copied files per spawn in the log. Report the denominator — `copied N of M candidates`, not `copied N` (#113). - A test that a member worktree gets exactly the configured overlay set and nothing else. ## Not verified I have not checked whether this repo's checkout actually has an `.envrc`, nor whether `direnv` is installed on this host. So this is a real defect in the mechanism, not a demonstrated live exposure. Establish that first — the answer changes the priority.
ltms commented 2026-08-23 07:54:17 +02:00
Author
Owner
Copy Link
Copy Source

Checked the "not verified" section myself before anyone acts on this:

  • No .env or .envrc exists in this checkout — not at the repo root, not in bridged/.
  • direnv is not installed on this host.

So there is no live exposure today. The overlay copies nothing, because there is nothing to copy, and even a copied .envrc would sit inert without direnv.

That drops the priority to low. It does not close the ticket: the default is still "copy an executable file we do not own into every member", and the day someone adds an .envrc for local convenience it becomes live with nothing warning them. That is the shape worth fixing while it is cheap.

Point 3 — logging which overlay files were copied — is worth doing on its own merit and independent of the decision on points 1 and 2. Right now the only reason I can tell you there is no exposure is that I went and looked at the filesystem; the daemon's logs say nothing either way.

Checked the "not verified" section myself before anyone acts on this: - **No `.env` or `.envrc` exists** in this checkout — not at the repo root, not in `bridged/`. - **`direnv` is not installed** on this host. So there is **no live exposure today**. The overlay copies nothing, because there is nothing to copy, and even a copied `.envrc` would sit inert without `direnv`. That drops the priority to low. It does not close the ticket: the default is still "copy an executable file we do not own into every member", and the day someone adds an `.envrc` for local convenience it becomes live with nothing warning them. That is the shape worth fixing while it is cheap. Point 3 — logging which overlay files were copied — is worth doing on its own merit and independent of the decision on points 1 and 2. Right now the only reason I can tell you there is no exposure is that I went and looked at the filesystem; the daemon's logs say nothing either way.
ltms commented 2026-09-03 06:01:33 +02:00
Author
Owner
Copy Link
Copy Source

Lead decision on points 1 and 2, with the facts first

The ticket says the live exposure was never established and that the answer changes the priority. I checked both hosts.

Check Mac fleet01
.env in the checkout absent absent
.envrc in the checkout absent absent
direnv on PATH not installed not installed
.env/.envrc in any live worktree none none

So there is no live exposure today on either host. This is a latent defect in the default, exactly as suspected. That sets the priority: fix it, but it is not urgent.

Point 1 — does the scrub need to cover direnv? No, out of scope.

Two reasons. First, direnv is not installed on either host, so this would be a control for a tool we do not run. Second, and this is the lasting reason: a direnv hook runs on every cd, so a one-shot scrub in .zlogin can never catch it. Building a cd-time scrub means owning a hook in a file the member controls — which is the very shape CB-633 exists to get away from. The right control is to not copy the executable file at all, which is point 2.

Point 2 — drop .envrc from the default? Yes.

.env is data. .envrc is executable shell that runs inside the member. Parity means a worker can reproduce the primary's build, and an executable hook is not needed for that. Dropping it removes the executable half and keeps the feature. The knob stays, so an operator who genuinely wants .envrc can still list it — the defect is the default, not the absence of a choice.

The default is one line: FleetConfig.java:411, List.of(".env", ".envrc") → List.of(".env").

I am holding this half back for now. FleetConfig.java is owned by #201/#227 Unit 5, which is in flight. I will make the one-line change myself once Unit 5 merges, rather than create a conflict in a central file for a latent defect. Tracking it here so it is not lost.

Point 3 — do it regardless, and it pairs with #134

GitWorktrees.overlayParity (GitWorktrees.java:483-508) logs every step at debug, so at the default level the copy is silent — which is what makes this hard to notice, as the ticket says. The same method also marks a copied tracked file --skip-worktree at line 503 and says nothing, which is #134. Same method, same silence, so I am delegating them as one unit. The log will report the denominator (copied N of M candidates) per #113.

## Lead decision on points 1 and 2, with the facts first The ticket says the live exposure was never established and that the answer changes the priority. I checked both hosts. | Check | Mac | fleet01 | |---|---|---| | `.env` in the checkout | absent | absent | | `.envrc` in the checkout | absent | absent | | `direnv` on PATH | not installed | not installed | | `.env`/`.envrc` in any live worktree | none | none | So there is **no live exposure today** on either host. This is a latent defect in the default, exactly as suspected. That sets the priority: fix it, but it is not urgent. ### Point 1 — does the scrub need to cover `direnv`? **No, out of scope.** Two reasons. First, `direnv` is not installed on either host, so this would be a control for a tool we do not run. Second, and this is the lasting reason: a `direnv` hook runs on every `cd`, so a one-shot scrub in `.zlogin` can never catch it. Building a `cd`-time scrub means owning a hook in a file the member controls — which is the very shape CB-633 exists to get away from. The right control is to not copy the executable file at all, which is point 2. ### Point 2 — drop `.envrc` from the default? **Yes.** `.env` is data. `.envrc` is executable shell that runs inside the member. Parity means a worker can reproduce the primary's build, and an executable hook is not needed for that. Dropping it removes the executable half and keeps the feature. The knob stays, so an operator who genuinely wants `.envrc` can still list it — the defect is the default, not the absence of a choice. The default is one line: `FleetConfig.java:411`, `List.of(".env", ".envrc")` → `List.of(".env")`. **I am holding this half back for now.** `FleetConfig.java` is owned by #201/#227 Unit 5, which is in flight. I will make the one-line change myself once Unit 5 merges, rather than create a conflict in a central file for a latent defect. Tracking it here so it is not lost. ### Point 3 — do it regardless, and it pairs with #134 `GitWorktrees.overlayParity` (`GitWorktrees.java:483-508`) logs every step at `debug`, so at the default level the copy is silent — which is what makes this hard to notice, as the ticket says. The same method also marks a copied tracked file `--skip-worktree` at line 503 and says nothing, which is **#134**. Same method, same silence, so I am delegating them as one unit. The log will report the denominator (`copied N of M candidates`) per #113.
ltms commented 2026-09-03 06:16:50 +02:00
Author
Owner
Copy Link
Copy Source

Point 3 is done — merged to main in 0d7b4fb (PR #242).

overlayParity now reports the outcome per spawn at info, with the denominator as this ticket and #113 asked:

parity overlay: copied 2 of 2 candidates: .env, .envrc
parity overlay: copied 1 of 2 candidates: .env (.envrc absent)
parity overlay marked --skip-worktree (cannot be committed from this worktree): .env

The copy and mark logic is byte-for-byte unchanged; only logging is new. Verified: 1168 tests, 0 failures; reverting either line back to log.debug goes red with 0 compile errors, so the fix cannot regress to an invisible level.

There is also a test that a member worktree gets exactly the configured overlay set and nothing else, which is the last acceptance line here.

Still open on this ticket

Point 2 only — dropping .envrc from the default at FleetConfig.java:411. Decided above, held back because FleetConfig.java is owned by #201/#227 Unit 5, which is still in flight. I will make that one-line change myself as soon as Unit 5 merges.

Point 1 was decided as out of scope, with reasons, in my earlier comment.

One correction

My PR comment originally paired this with #134. That was wrong: #134 is about isolateToolSurface, a different method. #148 point 3 is fully delivered here; #134 is reopened and being worked separately.

**Point 3 is done** — merged to main in `0d7b4fb` (PR #242). `overlayParity` now reports the outcome per spawn at `info`, with the denominator as this ticket and #113 asked: ``` parity overlay: copied 2 of 2 candidates: .env, .envrc parity overlay: copied 1 of 2 candidates: .env (.envrc absent) parity overlay marked --skip-worktree (cannot be committed from this worktree): .env ``` The copy and mark logic is byte-for-byte unchanged; only logging is new. Verified: 1168 tests, 0 failures; reverting either line back to `log.debug` goes red with 0 compile errors, so the fix cannot regress to an invisible level. There is also a test that a member worktree gets exactly the configured overlay set and nothing else, which is the last acceptance line here. ### Still open on this ticket **Point 2 only** — dropping `.envrc` from the default at `FleetConfig.java:411`. Decided above, held back because `FleetConfig.java` is owned by #201/#227 Unit 5, which is still in flight. I will make that one-line change myself as soon as Unit 5 merges. Point 1 was decided as out of scope, with reasons, in my earlier comment. ### One correction My PR comment originally paired this with #134. That was wrong: #134 is about `isolateToolSurface`, a different method. #148 point 3 is fully delivered here; #134 is reopened and being worked separately.
ltms commented 2026-09-03 06:50:44 +02:00
Author
Owner
Copy Link
Copy Source

Decision on points 1 and 2 (lead), with the evidence the ticket asked for first

Live exposure: none today. I checked this host on 2026-09-03:

  • ls -la .env .envrc in the primary checkout → both absent.
  • command -v direnv → not on PATH.

So the copy has nothing to copy and nothing would evaluate it. This is a defect in the default, not a demonstrated leak. That lowers the urgency but does not change the fix — the ticket's own note says creating either file at the repo root is enough to make it live, and that is a one-command accident.

Point 2 — yes, drop .envrc from the default. New default: [.env].

.env is data: a copy of it can only carry values. .envrc is executable shell that direnv runs on cd, so a copy of it can carry behaviour. Those are not the same risk and they do not deserve the same default. Parity is still the feature, so the knob stays: an operator who genuinely wants .envrc copied writes parityOverlay: [".env", ".envrc"] and owns that choice explicitly. Nothing is removed, only un-defaulted.

Point 1 — the scrub should not be extended to direnv. Out of scope, and it is the wrong layer.

The CB-633 scrub is a one-shot .zlogin under ZDOTDIR. A direnv hook runs on every cd, which is after the shell's startup files by definition. So a one-shot scrub can never cover it — not because we wrote it wrong, but because the two mechanisms run at different times, and no amount of work on the scrub closes that. Chasing it would mean owning a direnv hook of our own inside every member, which is a much larger surface than the thing it protects.

Point 2 makes point 1 unnecessary instead: if fleetd never copies the executable file, there is nothing for direnv to evaluate that fleetd put there. That is the smaller change and it removes the executable half, exactly as the ticket proposed.

One limit, stated plainly: this only covers .envrc files fleetd copies. A repo that has its own committed .envrc still gets it in every worktree, because a worktree is a checkout of the repo. Nothing here changes that, and nothing should — that file is the repo's, not ours. This repo has none.

Point 3 — already done

Shipped in 0d7b4fb. The overlay now logs at info with the denominator the ticket asked for:

parity overlay: copied 1 of 2 candidates: .env (.envrc absent)
parity overlay marked --skip-worktree (cannot be committed from this worktree): .env

Point 2 is next; I will close this ticket when it lands with its test.

## Decision on points 1 and 2 (lead), with the evidence the ticket asked for first **Live exposure: none today.** I checked this host on 2026-09-03: - `ls -la .env .envrc` in the primary checkout → both **absent**. - `command -v direnv` → **not on PATH**. So the copy has nothing to copy and nothing would evaluate it. This is a defect in the default, not a demonstrated leak. That lowers the urgency but does not change the fix — the ticket's own note says creating either file at the repo root is enough to make it live, and that is a one-command accident. ### Point 2 — yes, drop `.envrc` from the default. New default: `[.env]`. `.env` is data: a copy of it can only carry values. `.envrc` is **executable shell** that `direnv` runs on `cd`, so a copy of it can carry behaviour. Those are not the same risk and they do not deserve the same default. Parity is still the feature, so the knob stays: an operator who genuinely wants `.envrc` copied writes `parityOverlay: [".env", ".envrc"]` and owns that choice explicitly. Nothing is removed, only un-defaulted. ### Point 1 — the scrub should **not** be extended to `direnv`. Out of scope, and it is the wrong layer. The CB-633 scrub is a one-shot `.zlogin` under `ZDOTDIR`. A `direnv` hook runs on **every** `cd`, which is after the shell's startup files by definition. So a one-shot scrub can never cover it — not because we wrote it wrong, but because the two mechanisms run at different times, and no amount of work on the scrub closes that. Chasing it would mean owning a `direnv` hook of our own inside every member, which is a much larger surface than the thing it protects. Point 2 makes point 1 unnecessary instead: if fleetd never copies the executable file, there is nothing for `direnv` to evaluate that fleetd put there. That is the smaller change and it removes the executable half, exactly as the ticket proposed. **One limit, stated plainly:** this only covers `.envrc` files *fleetd copies*. A repo that has its own committed `.envrc` still gets it in every worktree, because a worktree is a checkout of the repo. Nothing here changes that, and nothing should — that file is the repo's, not ours. This repo has none. ### Point 3 — already done Shipped in `0d7b4fb`. The overlay now logs at `info` with the denominator the ticket asked for: ``` parity overlay: copied 1 of 2 candidates: .env (.envrc absent) parity overlay marked --skip-worktree (cannot be committed from this worktree): .env ``` Point 2 is next; I will close this ticket when it lands with its test.
ltms commented 2026-09-03 07:05:24 +02:00
Author
Owner
Copy Link
Copy Source

All three points are done. Closing.

Point Outcome Commit
1 — extend the credential scrub to direnv? Declined, with the reason recorded above: the scrub is a one-shot .zlogin and a direnv hook runs on every cd, so no work on the scrub can cover it. Point 2 removes the need instead. —
2 — drop .envrc from the default Done. The default is [".env"]. 43206ca
3 — log which overlay files were actually copied, with the denominator Done. 0d7b4fb

Verified: 1216 tests, 0 failures, 0 compile errors, BUILD SUCCESS.

The escape hatch is pinned by its own test — an operator writing parityOverlay: [".env", ".envrc"] still gets both, verbatim. Without that test this change would be a removal rather than a re-default, and nothing would notice if it silently became one.

Mutation check: reverting the default to List.of(".env", ".envrc") turns parityOverlayDefaultsToDotEnvOnly red, with 0 compile errors counted separately.

Two things the worker found and handled well:

  • WorktreeSessionManagerTest hardcoded the same default at another layer and broke the build. That is outside the file list I named in the brief. It fixed it and said so rather than folding it in quietly.
  • docs/CB-301-ext-Worktree-Provisioning.md and docs/Worker-Git-Workflow.md still describe the old [.env, .envrc] pair. Reported in one line and correctly left alone. Small doc drift, not worth a ticket on its own; whoever next edits those files should fix the sentence.

One limit worth restating, because this ticket does not remove it. This only covers .envrc files fleetd copies. A repo that commits its own .envrc still gets it in every worktree, because a worktree is a checkout of that repo. Nothing here changes that and nothing should — that file belongs to the repo, not to fleetd. This repo has none.

All three points are done. Closing. | Point | Outcome | Commit | |---|---|---| | 1 — extend the credential scrub to `direnv`? | **Declined**, with the reason recorded above: the scrub is a one-shot `.zlogin` and a `direnv` hook runs on every `cd`, so no work on the scrub can cover it. Point 2 removes the need instead. | — | | 2 — drop `.envrc` from the default | **Done.** The default is `[".env"]`. | `43206ca` | | 3 — log which overlay files were actually copied, with the denominator | **Done.** | `0d7b4fb` | Verified: 1216 tests, 0 failures, 0 compile errors, `BUILD SUCCESS`. The escape hatch is pinned by its own test — an operator writing `parityOverlay: [".env", ".envrc"]` still gets both, verbatim. Without that test this change would be a removal rather than a re-default, and nothing would notice if it silently became one. Mutation check: reverting the default to `List.of(".env", ".envrc")` turns `parityOverlayDefaultsToDotEnvOnly` red, with 0 compile errors counted separately. Two things the worker found and handled well: - `WorktreeSessionManagerTest` hardcoded the same default at another layer and broke the build. That is outside the file list I named in the brief. It fixed it and **said so** rather than folding it in quietly. - `docs/CB-301-ext-Worktree-Provisioning.md` and `docs/Worker-Git-Workflow.md` still describe the old `[.env, .envrc]` pair. Reported in one line and correctly left alone. Small doc drift, not worth a ticket on its own; whoever next edits those files should fix the sentence. **One limit worth restating, because this ticket does not remove it.** This only covers `.envrc` files *fleetd copies*. A repo that commits its own `.envrc` still gets it in every worktree, because a worktree is a checkout of that repo. Nothing here changes that and nothing should — that file belongs to the repo, not to fleetd. This repo has none.
ltms closed this issue 2026-09-03 07:05:26 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#148
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?