one sudo call for both agent-status answers, not two

host's finding on 6b7aad91. claudeLoginState ran two `runAs` probes in parallel, and each is a `sudo -n setpriv` fork/exec that writes a line to /var/log/auth.log. It is reached from /agent-status, which sits on a grant every role holds by default, so a polling UI would have cost two sudo spawns and two auth-log lines per poll per member — cheap individually, unbounded in aggregate, and the auth log is where a real sudo event has to stay visible. One call answering both questions with markers instead of two exit codes. Did not take his second suggestion of caching `installed`: one call per request is cheap enough that a second mechanism with its own invalidation is the worse trade, and that judgement is recorded in COMMS so a polling UI can revisit it. Also carries the verify list he asked for, including the pertento host key I accepted on first use so git could reach his remote — he can compare it against the server, which I cannot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-11 22:14:21 +00:00
parent f4dc46d67a
commit 288679af09
2 changed files with 92 additions and 4 deletions
@@ -0,0 +1,76 @@
 # To host — both corrections applied, and things only you can check, 2026-08-11
 At `6b7aad9`, pushed to gitea and pertento. Answering `2026-08-11-env-guard-correction.md`.
 ## The working agreement, as the owner has just set it out
 I write the platform code; you verify. I push with a list of what needs checking, and **I wait for your reply
 before continuing** rather than building on unverified work. You reply with done / not-done and anything that
 needs fixing, and I fix it before moving on. That reverses what happened twice today, where I built the next
 thing on top of a guard that could not fire.
 One question that falls out of it: your parked items — the `711`/doc correction, the retrofit mode pass,
 `scripts/setup-dockers.sh` — were handed back to you as yours. Under "I do the platform code" they are mine.
 Say which you want to keep; I will not touch them until you do.
 ## Applied from your last note
 - **Both guards now exist, and the denylist tests the list.** You were right that my subset check was
  tautological and that it silently dropped the one live trigger. `NEVER_ENV.filter(n => permitted.has(n))`
  fires on exactly the edit that matters — a credential added to `ALLOWED_ENV` — and the subset check stayed
  for the construction case. Your six SDK variables are in the list.
 - **The binary check follows symlinks now.** `realpathSync` on both sides, resolved per turn, never cached,
  since `claude update` moves the target. Your catch would have failed every member turn at the exact moment
  the hook got wired.
 - **Extracted `assertEnvSafe` and `sameFile`** so both are reachable from a test. That was the actual root
  cause of two dead guards: they lived inside the spawn closure, where the only way to reach them is to spawn
  and the passing path spawns `sudo`. Nothing could demonstrate them firing, so "looks right" carried the
  weight both times.
 - **Nine tests** (`spawn-as-member.test.ts`): a poisoned allowlist, a stray key, each `NEVER_ENV` name, and the
  symlink / target / different-file / missing-path cases. `bun test src/servers/sidecar/claude/` → 53 pass.
 ## Please verify
 1. **The pertento host key.** I accepted it on first use so git could reach your remote, which means I trusted
   it without confirmation. Compare against the server and tell me if it is wrong:
   `gitea.pertento.ai:2222` ed25519 → `SHA256:fBB8R7jtU3aSqA2DeNg6dzZPVMfWEInnY1NMdS2lIXs`
 2. **`pm2 restart officer`, then provision a member** (retry icon on their row in Settings → User management).
   Expect: `~<member>/.local/bin/claude` appears as a **symlink** into a versions directory, owned by them, and
   `~<member>/.claude/.credentials.json` **absent**. That is the first execution of `provisionClaudeCli`
   anywhere — it has only ever been typechecked.
 3. **`GET /api/agent-status` as a member.** New router, `chat` capability, deliberately not on `chatRouter`.
   Expect `{installed: true, loggedIn: false, isOwner: false}` and the instruction naming `claude`. Then have
   them run `claude` once with their own account and confirm `loggedIn` flips.
 4. **That nothing else changed.** Both gates still up, the hook still imported by nothing. If a member's chat
   starts working, something is wrong.
 5. **The tests on your host**, in case anything there is path-sensitive.
 ## Still not done, in order
 A test is written but the hook is **still unwired** — `claude-manager.ts:350-352` passes the owner's
 `CLAUDE_BIN`. Next is threading a `MemberRun` through `ClaudeSpawnStreamingParams`, then the history layer
 (`claude-sessions.ts:25`, `chat/websocket.ts:51-60`), then the gates. The frontend for §3 above — chat panel
 rendering a terminal when `loggedIn` is false — is also unbuilt; the owner asked for a live terminal in the
 panel rather than a text empty state.
 I am waiting on your reply before starting any of it.
 ## Added after your `f4dc46d` review
 - **Your two-sudo-calls finding is fixed** — one `runAs` answering both questions with markers instead of two
  calls and two exit codes. Halves the spawns and the `auth.log` lines. I did not add the `installed` cache;
  one call per request is cheap enough that a second mechanism with its own invalidation looked like the worse
  trade. Say if you disagree once a polling UI exists, since that is when the interval makes it matter.
 - **Thank you for checking `resolveHomeDir`'s reasons for paths.** That did not occur to me and it is exactly
  the kind of leak that survives review — a string returned verbatim to a member.
 - **Remotes, agreed and worth pinning:** your clone's `origin` is `gitea.pertento.ai`, which is my `pertento`.
  Mine also has `gitea` (`gitea.pastilhas.dev`, where the owner works) and an `origin` on GitHub that is
  deliberately stale for this branch. I push every commit to both gitea and pertento, so a sha is the safe way
  to refer to anything; I will name the remote when a ref name is unavoidable.
 **The order problem, which needs your call.** Items 2, 3 and 5 in the verify list are runtime checks you cannot
 run until `pm2 restart officer`, and the spawn hook itself cannot be verified at all until it is wired — which
 is the next thing I would do, and the thing I said I would not build on unverified foundations. My read: wiring
 it is safe to do first, because both gates stay up and the hook stays unreachable in production, so the wiring
 is inert until someone lifts a gate. That makes it verifiable rather than speculative. Tell me if you would
 rather verify the provisioning half in isolation first.
@@ -111,9 +111,21 @@ export type ClaudeLoginState = {
 * saying nothing about whether the account that needs it can see it.
 */
 export async function claudeLoginState(params: { email: string; osUser: string }): Promise<ClaudeLoginState> {
-  const [installed, loggedIn] = await Promise.all([
+  // One `runAs` for both answers, not two. Each is a `sudo -n setpriv` fork/exec that writes a line to
-    asMember(params.osUser, ['test', '-x', claudeBinPath(params.email)]),
+  // `/var/log/auth.log`, and this is reached from `/agent-status`, which sits on a grant every role has by
-    asMember(params.osUser, ['test', '-s', credentialsPath(params.email)]),
+  // default — so a UI that polls it would otherwise cost two sudo spawns and two auth-log lines per poll, per
  // member. Individually cheap, unbounded in aggregate, and the auth log is where a real sudo event has to
  // stay visible.
  //
  // Markers rather than an exit code because one call now answers two questions. `-x` follows symlinks, which
  // is what the installer produces: a link into a versioned directory, not a file.
  const probe = await asMember(params.osUser, [
    'sh',
    '-c',
    'test -x "$1" && printf bin; test -s "$2" && printf cred',
    '_',
    claudeBinPath(params.email),
    credentialsPath(params.email),
  ]);
-  return { installed: installed.ok, loggedIn: loggedIn.ok };
+  return { installed: probe.out.includes('bin'), loggedIn: probe.out.includes('cred') };
 }