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

memberCredentials.sshAuthSock: rename the values block/allow to omit/inherit — "block" names something fleetd does not do #266

New Issue
Closed
opened 2026-09-03 15:05:20 +02:00 by ltms · 1 comment
ltms commented 2026-09-03 15:05:20 +02:00
Owner
Copy Link
Copy Source

Split out of #184. That ticket fixed the prose around this key. The key's own values are still wrong, and a value name is read far more often than the comment beside it.

The problem

memberCredentials.sshAuthSock takes block (default) or allow.

block does not block anything. It omits SSH_AUTH_SOCK from the member's environment. The member runs as the same OS user as fleetd, so it can still reach that socket — the path is discoverable, and nothing revokes access to it. Measured on 2026-08-28: a member with the variable blanked pushed to the forge over SSH successfully, because ssh -G resolves an IdentityFile that is readable and has no passphrase.

So the setting reduces accidental use of the operator's agent. It is not a deny.

Why the name matters more than the comment

An operator scanning a config file reads sshAuthSock: block and stops. That is the whole point of a good value name — it saves you reading the comment. Here it costs you: the name promises enforcement, the comment (correctly, since a2b8caf) explains there is none, and only the second one is true.

This is the same failure #184 exists to remove, one layer down. fleetd should not name a control after an effect it does not have.

Proposed

now proposed why
block omit says exactly what happens: the variable is left out of the member environment
allow inherit pairs with omit, and matches how every other env decision here is described

Consider renaming the key itself to sshAgentEnv at the same time, since the value is about the environment variable, not the socket. One rename is cheaper than two.

Requirements

  • Read-both shim. block and allow keep working, unchanged, with no warning on read. This key is in live configs; a rename that breaks a running fleet trades a naming bug for an outage.
  • Emit the canonical new form when the config is echoed or logged.
  • FleetConfig.sshAuthSockAllowed() is the single decision point (HerdrPeerLauncher:1474 is its only caller) — the shim belongs in normalisation, not spread across call sites.
  • Update fleetd.example.yaml, keeping the honest explanation a2b8caf added. Do not shorten it. The comment must still say that omitting the variable does not deny same-user access to the socket and does not affect readable key files on disk.
  • Tests for all four accepted spellings, and for the default when the key is absent.

Do not overcorrect

The naming fix must not turn into "this setting is pointless, drop it". It is worth keeping: it stops a member picking up the agent by default, which is a real reduction in accidents. Say what it does and what it does not do — do not delete the reason it exists. See the same trap on #184.

Not in scope

Any change to what the setting actually does. A real boundary needs a different OS user or OS-level confinement, which is #185.

Split out of #184. That ticket fixed the *prose* around this key. The key's own values are still wrong, and a value name is read far more often than the comment beside it. ## The problem `memberCredentials.sshAuthSock` takes `block` (default) or `allow`. `block` does not block anything. It omits `SSH_AUTH_SOCK` from the member's environment. The member runs as the same OS user as `fleetd`, so it can still reach that socket — the path is discoverable, and nothing revokes access to it. Measured on 2026-08-28: a member with the variable blanked pushed to the forge over SSH successfully, because `ssh -G` resolves an `IdentityFile` that is readable and has no passphrase. So the setting reduces **accidental** use of the operator's agent. It is not a deny. ## Why the name matters more than the comment An operator scanning a config file reads `sshAuthSock: block` and stops. That is the whole point of a good value name — it saves you reading the comment. Here it costs you: the name promises enforcement, the comment (correctly, since `a2b8caf`) explains there is none, and only the second one is true. This is the same failure #184 exists to remove, one layer down. `fleetd` should not name a control after an effect it does not have. ## Proposed | now | proposed | why | |---|---|---| | `block` | `omit` | says exactly what happens: the variable is left out of the member environment | | `allow` | `inherit` | pairs with `omit`, and matches how every other env decision here is described | Consider renaming the key itself to `sshAgentEnv` at the same time, since the value is about the *environment variable*, not the socket. One rename is cheaper than two. ## Requirements - **Read-both shim.** `block` and `allow` keep working, unchanged, with no warning on read. This key is in live configs; a rename that breaks a running fleet trades a naming bug for an outage. - Emit the canonical new form when the config is echoed or logged. - `FleetConfig.sshAuthSockAllowed()` is the single decision point (`HerdrPeerLauncher:1474` is its only caller) — the shim belongs in normalisation, not spread across call sites. - Update `fleetd.example.yaml`, keeping the honest explanation `a2b8caf` added. **Do not shorten it.** The comment must still say that omitting the variable does not deny same-user access to the socket and does not affect readable key files on disk. - Tests for all four accepted spellings, and for the default when the key is absent. ## Do not overcorrect The naming fix must not turn into "this setting is pointless, drop it". It is worth keeping: it stops a member picking up the agent by default, which is a real reduction in accidents. Say what it does and what it does not do — do not delete the reason it exists. See the same trap on #184. ## Not in scope Any change to what the setting actually does. A real boundary needs a different OS user or OS-level confinement, which is #185.
ltms commented 2026-09-04 03:26:58 +02:00
Author
Owner
Copy Link
Copy Source

Done — merged as 9020d01 (PR #268). Closing.

All eight spellings are accepted: sshAuthSock and sshAgentEnv, each with block/allow/omit/inherit. sshAgentEnv wins if both keys are present. An unrecognised value normalises to omit, so it still fails closed.

Verified on the live config, which is the part no test could cover. fleetd.yaml on this host is gitignored and still uses the old spelling sshAuthSock: block, so the shim is load-bearing here, not cosmetic. After redeploying the daemon on the new jar:

  • GET /member-credentials → policy: allow-list, 39 known / 7 allowed / 34 blocked — the same shape as before the change, so the config parsed and the policy survived.
  • A real member spawned on the new jar reported SSH_AUTH_SOCK: unset and LAVINMQ_URI: unset, with AI_GATEWAY_TOKEN and WORKER_GITEA_TOKEN set (both are on allow: by design).

So the old spelling still resolves to "omit" end to end, in the running daemon, not just in a test.

The example comment kept its full explanation — that omitting the variable does not deny same-user access to the socket, and does not affect readable key files on disk. That was the trap on this ticket and it was avoided.

Done — merged as `9020d01` (PR #268). Closing. All eight spellings are accepted: `sshAuthSock` and `sshAgentEnv`, each with `block`/`allow`/`omit`/`inherit`. `sshAgentEnv` wins if both keys are present. An unrecognised value normalises to `omit`, so it still fails closed. **Verified on the live config, which is the part no test could cover.** `fleetd.yaml` on this host is gitignored and still uses the old spelling `sshAuthSock: block`, so the shim is load-bearing here, not cosmetic. After redeploying the daemon on the new jar: - `GET /member-credentials` → `policy: allow-list`, 39 known / 7 allowed / 34 blocked — the same shape as before the change, so the config parsed and the policy survived. - A real member spawned on the new jar reported `SSH_AUTH_SOCK: unset` and `LAVINMQ_URI: unset`, with `AI_GATEWAY_TOKEN` and `WORKER_GITEA_TOKEN` set (both are on `allow:` by design). So the old spelling still resolves to "omit" end to end, in the running daemon, not just in a test. The example comment kept its full explanation — that omitting the variable does not deny same-user access to the socket, and does not affect readable key files on disk. That was the trap on this ticket and it was avoided.
ltms closed this issue 2026-09-04 03:27:01 +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#266
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?