From 0331ecd5d312aa5043a4c61dba2233e7e6c81e75 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 18:37:52 +0200 Subject: [PATCH] =?UTF-8?q?CB-592:=20add=20the=20BRIDGED=5FMEMBER=20marker?= =?UTF-8?q?=20=E2=80=94=20the=20sentinel=20alone=20cannot=20hold?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live check on a member pane showed the CB-592 shadow did NOT take effect: GITEA_ACCESS_TOKEN inside the pane was still the real admin token. Measured cause. The overlay itself works — GITEA_TOKEN is injected the same way, is exported by no shell file, and does reach the pane. The sentinel loses one step later. A herdr pane runs a LOGIN shell, ~/.zprofile line 41 sources ${SHARED_ENV}/tools/secrets.sh, and that file does a plain unconditional `export GITEA_ACCESS_TOKEN=...`. A login shell overwrites a value already in the environment, so the real token is put back before the member starts. Confirmed directly: GITEA_ACCESS_TOKEN=cb592-sentinel zsh -lc ... -> RESULT: sentinel was OVERWRITTEN by the login shell This defeats any launcher-side overlay for any name secrets.sh exports. No change in this repo can win it alone. So this adds the half that does survive: BRIDGED_MEMBER=1, a name secrets.sh never exports. It is a no-op until the operator guards the export: [ -n "${BRIDGED_MEMBER:-}" ] || export GITEA_ACCESS_TOKEN=... Setting it now costs nothing and makes that one line the whole remaining fix. The sentinel stays: it is correct for any peer kind whose pane does not start a login shell, and it keeps the intent explicit where every adapter passes. Also corrects the javadoc and the test javadoc, which both claimed a protection that was measured not to hold. The other reported failure was my own bad test, not a regression. The probe called /api/v1/user, which a minimal write:repository token cannot read. Same token on the repo endpoint answers 200, so CB-302 is intact: GITEA_ACCESS_TOKEN: /user=200 /repos/lms/claude-bridge=200 WORKER_GITEA_TOKEN: /user=403 /repos/lms/claude-bridge=200 Tests 805 -> 807. Both new tests proved to discriminate by reverting the marker: everySpawnMarksThePaneAsAMember and aProfileEnvEntryCannotClearTheMemberMarker both fail without it. Refs: gitea #77 --- .../bridged/member/HerdrPeerLauncher.java | 43 +++++++++++++++++-- .../member/ClaudeCodeLauncherTest.java | 39 +++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java index af0470e..ee3ab20 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java @@ -781,10 +781,43 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * PATH} seeding already depends on the overlay reliably replacing an inherited value (see its * javadoc), and that is only demonstrated for a non-blank value, so this reuses the same, * proven-reliable shape rather than the unverified one. + * + *

MEASURED ON A LIVE PANE, 2026-08-15: this sentinel alone does NOT hold. The overlay + * itself works — {@code GITEA_TOKEN} is injected here, is exported by no shell file, and does + * reach the pane. The sentinel loses one step later. A herdr pane runs a login shell, + * {@code ~/.zprofile} sources {@code ${SHARED_ENV}/tools/secrets.sh}, and that file does a plain + * unconditional {@code export GITEA_ACCESS_TOKEN=...}. A login shell overwrites a value already + * in the environment, so the real admin token is put back over this sentinel before the member + * process ever starts. That defeat applies to every name {@code secrets.sh} exports, + * and no launcher-side overlay can win against it. + * + *

So this constant is not the control on its own — {@link #MEMBER_MARKER} is the other half. + * Keeping the sentinel is still worth it: it is correct for any peer kind whose pane does not + * start a login shell, and it makes the intent explicit at the one place every adapter passes. */ private static final String BLOCKED_GITEA_ACCESS_TOKEN = "blocked-by-bridged-cb592-see-gitea-issue-77"; + /** + * CB-592: marks a pane as a bridged member so a shell startup file can decline to export + * operator-only credentials into it (gitea issue #77). + * + *

This name is deliberately one that {@code secrets.sh} never exports, which is exactly why + * it survives the login shell that wipes {@link #BLOCKED_GITEA_ACCESS_TOKEN}. The mechanism is + * measured, not assumed: {@code GITEA_TOKEN} is injected the same way, is absent from a login + * shell of its own, and was observed set inside a live member pane. + * + *

It is a no-op until the operator guards the export, which is a one-line change in a file + * this repo does not own and must not edit unasked: + * + * 

{@code
+     * [ -n "${BRIDGED_MEMBER:-}" ] || export GITEA_ACCESS_TOKEN=...
+     * }
+ * + *

Setting the marker now costs nothing and means that edit is the whole remaining fix. + */ + static final String MEMBER_MARKER = "BRIDGED_MEMBER"; + /** * Seed a worker's environment (CB-511): the daemon's own {@code PATH}, then the profile's * {@code env:} entries, then the CB-592 admin-token shadow. @@ -802,10 +835,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * dev.ltms.bridged.guard.SubscriptionGuard}, which is checked against the profile's * {@code baseUrl} and nothing else. * - *

The CB-592 shadow is put in last, after the profile's own {@code env:}, so no - * profile — present or future — can restore the admin token by naming it in config. This is - * the one place the shadow is applied: every {@code buildLaunch} in every adapter calls this - * first, so a new profile, and a peer kind not yet written, gets it for free. + *

The CB-592 shadow and marker are put in last, after the profile's own + * {@code env:}, so no profile — present or future — can restore the admin token, or hide that + * the pane is a member, by naming either in config. This is the one place both are applied: + * every {@code buildLaunch} in every adapter calls this first, so a new profile, and a peer + * kind not yet written, gets them for free. */ protected Map baseEnv(BridgedConfig.Profile cfg) { Map workerEnv = new LinkedHashMap<>(); @@ -817,6 +851,7 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { workerEnv.putAll(cfg.env()); } workerEnv.put("GITEA_ACCESS_TOKEN", BLOCKED_GITEA_ACCESS_TOKEN); + workerEnv.put(MEMBER_MARKER, "1"); return workerEnv; } diff --git a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java index 9b77a03..b2f015e 100644 --- a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java @@ -685,6 +685,12 @@ class ClaudeCodeLauncherTest { * explicit (non-blank) GITEA_ACCESS_TOKEN to herdr on every spawn, whatever the profile is, so * a future baseEnv refactor cannot silently drop it and reopen the leak. Asserted against what * tab.create's params actually carry, not an internal map built in the test (gitea #77). + * + *

Scope, measured on a live pane 2026-08-15: this pins what the launcher SENDS, and that is + * all it can pin. It does not prove the value survives, and it does not: the pane runs a login + * shell, ~/.zprofile sources secrets.sh, and its unconditional `export GITEA_ACCESS_TOKEN=...` + * puts the real token back over this sentinel. Closing that needs the operator to guard the + * export on BRIDGED_MEMBER — see everySpawnMarksThePaneAsAMember below. */ @Test void everySpawnShadowsTheAdminGiteaAccessToken() { @@ -716,6 +722,39 @@ class ClaudeCodeLauncherTest { "a profile's own env: must not be able to smuggle the admin token back in"); } + /** + * The half of CB-592 that can actually survive the pane's login shell. BRIDGED_MEMBER is a name + * secrets.sh never exports, so nothing overwrites it — measured: GITEA_TOKEN is injected the + * same way, is absent from a login shell of its own, and was observed set inside a live member + * pane. It lets the operator guard the admin export with + * `[ -n "${BRIDGED_MEMBER:-}" ] || export GITEA_ACCESS_TOKEN=...`, which is the whole fix. + * Pinned here so a refactor cannot drop the marker and quietly un-guard every member (#77). + */ + @Test + void everySpawnMarksThePaneAsAMember() { + FakeHerdr herdr = new FakeHerdr(); + service(herdr, List.of("claude"), null).spawn(); + + assertEquals("1", startEnv(herdr).get("BRIDGED_MEMBER"), + "every member pane must be marked, or a shell file cannot tell it apart from the operator's"); + } + + /** A profile must not be able to hide that its pane is a member, for the same reason as above. */ + @Test + void aProfileEnvEntryCannotClearTheMemberMarker() { + FakeHerdr herdr = new FakeHerdr(); + BridgedConfig.Profile cfg = new BridgedConfig.Profile( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "w #{n}", null, null, null, null, null, + null, Map.of("BRIDGED_MEMBER", ""), null, null); + new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + _ -> null).spawn(); + + assertEquals("1", startEnv(herdr).get("BRIDGED_MEMBER"), + "a profile's own env: must not be able to unmark its pane"); + } + // ── CB-533: the model is pinned on the command line, not only in the environment ──────────── /** A launcher for a profile identical but for its {@code model:} — the only variable here. */