From 288679af09c351456529493ab3139e0d3ab4cd71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 11 Aug 2026 22:14:21 +0000 Subject: [PATCH] one sudo call for both agent-status answers, not two MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../2026-08-11-verify-requests.md | 76 +++++++++++++++++++ src/servers/os-user-claude.ts | 20 ++++- 2 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 COMMS/sidecar-app-store/2026-08-11-verify-requests.md diff --git a/COMMS/sidecar-app-store/2026-08-11-verify-requests.md b/COMMS/sidecar-app-store/2026-08-11-verify-requests.md new file mode 100644 index 00000000..36d0ec6c --- /dev/null +++ b/COMMS/sidecar-app-store/2026-08-11-verify-requests.md @@ -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: `~/.local/bin/claude` appears as a **symlink** into a versions directory, owned by them, and + `~/.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. diff --git a/src/servers/os-user-claude.ts b/src/servers/os-user-claude.ts index e237fb2a..c902b0e4 100644 --- a/src/servers/os-user-claude.ts +++ b/src/servers/os-user-claude.ts @@ -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 { - const [installed, loggedIn] = await Promise.all([ - asMember(params.osUser, ['test', '-x', claudeBinPath(params.email)]), - asMember(params.osUser, ['test', '-s', credentialsPath(params.email)]), + // One `runAs` for both answers, not two. Each is a `sudo -n setpriv` fork/exec that writes a line to + // `/var/log/auth.log`, and this is reached from `/agent-status`, which sits on a grant every role has by + // 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') }; }