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

GITEA_HOST carries a scheme and a trailing slash, and every member builds URLs from it #377

New Issue
Closed
opened 2026-09-08 23:10:21 +02:00 by ltms · 1 comment
ltms commented 2026-09-08 23:10:21 +02:00
Owner
Copy Link
Copy Source

What happened

The fleet01 lead probed its merge permission on akb/kb and got HTTP 000. The cause was the URL it built:

https://https://git.ltms.dev//api/v1/repos/akb/kb/pulls/53/merge

It had written https://$GITEA_HOST/api/..., and GITEA_HOST already carries the scheme. The trailing slash then added the double slash.

This is not fleet01-specific

I checked the shape of the value on the Mac, without printing it:

GITEA_HOST: set (21 chars)
  starts with a scheme: yes
  trailing slash: yes

So both hosts carry the same shape, and fleetd passes it to every member: HerdrPeerLauncher.java:1183 does putIfPresent(workerEnv, "GITEA_HOST", resolveEnv(cfg.gitHostEnv())). It forwards the operator's value as it is, which I think is correct — normalising a value on the way through would hide what the operator actually set.

So there is no bug in the daemon's code here. The problem is that the name says HOST and the value is a URL with a trailing slash, and nothing tells a member that.

Why it is worth a ticket anyway

A member writing https://$GITEA_HOST/... produces a request that never leaves the machine. If the member's check is "did I get a 200?", a transport failure is indistinguishable from being refused. That is how a permission probe returns a confident wrong answer: the lead nearly concluded "still refused" from a request that never reached the server.

That failure is worse than a plain 404, because it is silent and it points at the wrong cause.

Directions (not a decision)

  • Document the shape where a member will actually read it — the member-facing surface, not a wiki page members do not see (see the note about members reading a months-old wiki).
  • Or rename the key so the name matches the value, which is a breaking config change and needs its own discussion.
  • Or have fleetd log the shape once at startup, the way it already logs startup secret <NAME>: set (...) without printing values.

Do not "fix" this by stripping the scheme in the launcher. That changes the value an operator set, on its way to a member, which is exactly the kind of silent rewrite that makes a config hard to reason about.

Related

  • #376 — the other way a probe returns a confident wrong answer (a fast reply read as a crash).
## What happened The fleet01 lead probed its merge permission on `akb/kb` and got `HTTP 000`. The cause was the URL it built: ``` https://https://git.ltms.dev//api/v1/repos/akb/kb/pulls/53/merge ``` It had written `https://$GITEA_HOST/api/...`, and `GITEA_HOST` already carries the scheme. The trailing slash then added the double slash. ## This is not fleet01-specific I checked the shape of the value on the Mac, without printing it: ``` GITEA_HOST: set (21 chars) starts with a scheme: yes trailing slash: yes ``` So both hosts carry the same shape, and `fleetd` passes it to every member: `HerdrPeerLauncher.java:1183` does `putIfPresent(workerEnv, "GITEA_HOST", resolveEnv(cfg.gitHostEnv()))`. It forwards the operator's value as it is, which I think is correct — normalising a value on the way through would hide what the operator actually set. So there is no bug in the daemon's code here. The problem is that the name says `HOST` and the value is a URL with a trailing slash, and nothing tells a member that. ## Why it is worth a ticket anyway A member writing `https://$GITEA_HOST/...` produces a request that never leaves the machine. If the member's check is "did I get a 200?", a transport failure is indistinguishable from being refused. That is how a permission probe returns a confident wrong answer: the lead nearly concluded "still refused" from a request that never reached the server. That failure is worse than a plain 404, because it is silent and it points at the wrong cause. ## Directions (not a decision) - Document the shape where a member will actually read it — the member-facing surface, not a wiki page members do not see (see the note about members reading a months-old wiki). - Or rename the key so the name matches the value, which is a breaking config change and needs its own discussion. - Or have `fleetd` log the shape once at startup, the way it already logs `startup secret <NAME>: set (...)` without printing values. Do not "fix" this by stripping the scheme in the launcher. That changes the value an operator set, on its way to a member, which is exactly the kind of silent rewrite that makes a config hard to reason about. ## Related - #376 — the other way a probe returns a confident wrong answer (a fast reply read as a crash).
ltms commented 2026-09-09 02:53:50 +02:00
Author
Owner
Copy Link
Copy Source

Merged to main in 2830735. No PR — the member could not create one (see #381).

How this one went

The gx member wrote the change and then could not run a single shell command. It asked me what to do rather than guessing, and its report said plainly: "build: not run — no shell. I have not compiled this code. I know that, and I do not claim otherwise." I committed its work from its worktree and took the build myself.

The production code was right. The tests were not.

First real build: 7 of its 9 tests failed. Nothing was wrong with Fleetd.java.

logback-test.xml sets dev.ltms.fleet to WARN. The new shape lines log at INFO, so logback's level check dropped every event before any appender saw it — the test's ListAppender captured an empty list, and every assertion about a logged line failed. The two tests that passed were the two pure ones (startsWithScheme, gitHostEnvVars), which touch no logging.

The member had copied attach()/detach() from MemberTrustModelReportTest, but that class does logger.setLevel(Level.INFO) at each call site, not inside the helper. Copying the helper alone leaves the level behind. Fixed by moving the level handling into attach()/detach(), which covers all nine at once, restoring the original level as null — meaning "inherit" — rather than a concrete level, because null is the state it starts in.

Worth noting: the feature itself is not inert. Production logback.xml sets dev.ltms.fleet to DEBUG, so these INFO lines do appear on a real daemon. Only the test config filtered them. If production had also been at WARN, this would have shipped as a feature that logs nothing, and the tests would still have failed — which is the useful thing about them failing loudly.

Mutation

The one property here worth pinning is that the value never reaches the log. I made the code log it:

varName, String.join(", ", sources) + " value=" + value,

Tests run: 9, Failures: 4
theValueNeverAppearsInLogOutput:
  the full GITEA_HOST value must never reach the log ==> expected: <false> but was: <true>

Restored and confirmed byte-identical. Full build after the fix: MVN_EXIT=0, Tests run: 1459, Failures: 0, 0 compile errors.

What the change does

At startup, next to the existing startup secret lines, for every profile that opted in via gitTokenEnv:

startup git host GITEA_HOST: set (profile 'local' gitHostEnv) — length=25, startsWithScheme=true, trailingSlash=true
startup git host GITEA_HOST: unset (profile 'local' gitHostEnv) — a member gets GITEA_TOKEN but no GITEA_HOST value
startup git host: no profile sets a gitTokenEnv — nothing to check

Shape only, never the value. The value still reaches members unchanged — rewriting it here would change what works on one host and breaks on another.

The member gated this on hasGitToken() for a reason worth keeping: HerdrPeerLauncher.applyGitToken (HerdrPeerLauncher.java:1183) injects GITEA_HOST only for profiles that opted in, so reporting unconditionally would describe a variable no member ever receives.

Still open after this, by design

This makes the daemon say what shape the value has. It does not stop a member building https://https://…. The member flagged that itself and did not chase it. That is a separate ticket, and this one does not claim to close it.

Two things it noticed outside scope, not acted on

  • deploy/fleetd.service:30-32 tells the operator to journalctl | grep 'startup secret'. The new line uses the startup git host prefix, so that grep will not show it.
  • fleetd.example.yaml:352-353 documents gitHostEnv and could use a line saying its shape is reported at startup.

Closing.

Merged to `main` in 2830735. No PR — the member could not create one (see #381). ## How this one went The gx member wrote the change and then could not run a single shell command. It asked me what to do rather than guessing, and its report said plainly: *"build: not run — no shell. I have not compiled this code. I know that, and I do not claim otherwise."* I committed its work from its worktree and took the build myself. ## The production code was right. The tests were not. First real build: **7 of its 9 tests failed.** Nothing was wrong with `Fleetd.java`. `logback-test.xml` sets `dev.ltms.fleet` to `WARN`. The new shape lines log at `INFO`, so logback's level check dropped every event **before any appender saw it** — the test's `ListAppender` captured an empty list, and every assertion about a logged line failed. The two tests that passed were the two pure ones (`startsWithScheme`, `gitHostEnvVars`), which touch no logging. The member had copied `attach()`/`detach()` from `MemberTrustModelReportTest`, but that class does `logger.setLevel(Level.INFO)` at each **call site**, not inside the helper. Copying the helper alone leaves the level behind. Fixed by moving the level handling into `attach()`/`detach()`, which covers all nine at once, restoring the original level as `null` — meaning "inherit" — rather than a concrete level, because `null` is the state it starts in. Worth noting: **the feature itself is not inert.** Production `logback.xml` sets `dev.ltms.fleet` to `DEBUG`, so these INFO lines do appear on a real daemon. Only the test config filtered them. If production had also been at `WARN`, this would have shipped as a feature that logs nothing, and the tests would still have failed — which is the useful thing about them failing loudly. ## Mutation The one property here worth pinning is that the value never reaches the log. I made the code log it: ```java varName, String.join(", ", sources) + " value=" + value, ``` ``` Tests run: 9, Failures: 4 theValueNeverAppearsInLogOutput: the full GITEA_HOST value must never reach the log ==> expected: <false> but was: <true> ``` Restored and confirmed byte-identical. Full build after the fix: `MVN_EXIT=0`, `Tests run: 1459, Failures: 0`, 0 compile errors. ## What the change does At startup, next to the existing `startup secret` lines, for every profile that opted in via `gitTokenEnv`: ``` startup git host GITEA_HOST: set (profile 'local' gitHostEnv) — length=25, startsWithScheme=true, trailingSlash=true startup git host GITEA_HOST: unset (profile 'local' gitHostEnv) — a member gets GITEA_TOKEN but no GITEA_HOST value startup git host: no profile sets a gitTokenEnv — nothing to check ``` Shape only, never the value. The value still reaches members unchanged — rewriting it here would change what works on one host and breaks on another. The member gated this on `hasGitToken()` for a reason worth keeping: `HerdrPeerLauncher.applyGitToken` (`HerdrPeerLauncher.java:1183`) injects `GITEA_HOST` **only** for profiles that opted in, so reporting unconditionally would describe a variable no member ever receives. ## Still open after this, by design This makes the daemon say what shape the value has. It does **not** stop a member building `https://https://…`. The member flagged that itself and did not chase it. That is a separate ticket, and this one does not claim to close it. ## Two things it noticed outside scope, not acted on - `deploy/fleetd.service:30-32` tells the operator to `journalctl | grep 'startup secret'`. The new line uses the `startup git host` prefix, so that grep will not show it. - `fleetd.example.yaml:352-353` documents `gitHostEnv` and could use a line saying its shape is reported at startup. Closing.
ltms closed this issue 2026-09-09 02:53:55 +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#377
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?