hyp status names the CA's permitted hosts, uninstall clears every duplicate root (#793) - #800
Conversation
…licate root (#793) Three findings from #793, plus the doc omission it names. - `hyp status` reported the CA fingerprint but not the hosts the grant covers, which LLP 0238's Consequences require ("the dialog and `hyp status` must name all permitted hosts, so the grant is informed"). The set is read off the certificate's own permitted subtrees, so it cannot drift from what the keychain actually vouches for. - `removeCaTrust` deleted one certificate per call while every HypAware CA carries the same common name, so a re-minted machine kept trusting the older roots after an uninstall that reported success. Removal now sweeps in a bounded loop until the keychain reports no match left, and says so if it hits the bound. - `runServiceCommand` had no timeout, and `hyp status` on darwin shells out through it; a locked login keychain can put `security` behind a GUI prompt with nobody watching. The helper takes an opt-in `timeoutMs`, the two read-only probes accept one, and status sets it. A timed-out probe rejects rather than returning non-zero, so it renders as `unknown` and never as a false "not trusted". - README and docs/PRIVACY.md now state the macOS launchd residue: attach runs `launchctl setenv NODE_USE_SYSTEM_CA 1` and installs a login LaunchAgent that re-applies it. F1 (`-p ssl` on the trust grant) and the F3 lifecycle residue stay open: both need a real Mac and an acceptance pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the LaunchAgent runs The permitted-host line is the one field of the proxy-trust report that is bytes off disk: a dNSName is an IA5String read out of whatever certificate sits at the CA path and decoded as latin1, with no charset or length check on the way. Our own mint refuses a non-printable host, but the renderer's 'not host-limited' arm exists precisely because a foreign certificate there is reachable, so a crafted one could carry an ESC run or a newline into a line hyp status prints. Sanitized at collection, like every other label in that file, and the JSDoc that claimed no LLP 0225 sanitizing applied here is corrected rather than left to mislead the next reader. PRIVACY.md said the login LaunchAgent 'starts no process of its own'. It execs /bin/launchctl once per login (buildEnvAgentPlist), which is what the code comment says and what a privacy document should say too.
VerdictApprove with two fixes pushed. The three behavioural changes are sound and I could not break any of the three load-bearing claims the PR body makes. Two findings, both low, both fixed on this branch ( Reviewed at head What I verified, rather than took on trustThe 8-pass sweep is safe ( Rejecting on timeout really is load-bearing ( No leaked child and no double-settle: SIGKILL on expiry,
Every README/PRIVACY claim, checked against the cited code. Findings1.
|
… surface can be flooded by them `hyp status` was taught to strip control bytes out of the certificate's own dNSName subtrees before printing them. The line the attach adapter writes immediately before macOS raises its password dialog names the same bytes from the same `readLocalCaInfo` read, and was not, so an ESC run or a newline in a subtree could repaint the sentence a user grants trust on the strength of. Neither surface bounded the *count* either, which is the third way one of these values is hostile and the one `recentEntrypointsFromSources` already answers a few hundred lines above the status collector. Both are now one shared policy, `displayableCaHosts`, next to the read that produces the bytes. It strips (label plane), names an entry that sanitizes away rather than dropping it, and ends a list longer than any real CA's with a count of what it left out - because both callers exist to state how wide a trust grant is, and a silently shortened list understates exactly that. LLP 0225#scope leaves `hyp status` and the client attach adapters to make the strip-versus-escape argument for themselves; this is that argument. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
VerdictApprove with one fix pushed. Round 2 of 2, reviewed at head
New finding1. The sanitize fix stopped one surface short, and left the third hostile dimension open on both - low, fixedRound 1 fixed a) The attach dialog was still unsanitized. ` Requesting keychain trust for the HypAware Local CA (limited to: ${hosts.join(', ')}).\n`Those Worth noting the corpus is explicit about this: LLP 0225#scope names b) Neither surface bounded the count. Fixed by lifting the policy into one shared exported helper next to the read that produces the bytes - On round 2's explicit question - does Verified as real guards, not decoration. Five new tests in Also swept up while in Round 1's three open nits - all re-examined, all still left2. Exactly 8 duplicates produce a false residue warning - nit, left (agreeing with round 1)
3. The two trust probes run serially, so
|
|
Triage at head |
Two conflicts, both from #818 (LLP 0262) moving Claude Code off proxy capture onto OTEL telemetry attach. hypaware-core/plugins-workspace/claude/src/index.js: resolved to master. This branch's only change there sanitized the permitted-host list in `ensureDarwinProxyTrust`'s keychain dialog, and #818 deleted both that function and its call site - `hyp attach claude` no longer mints or trusts a CA, so the dialog it hardened does not exist. The shared half of that work survives: `displayableCaHosts` still lives in src/core/tls/ca.js and `hyp status` still routes the hosts through it. README.md: kept master's client-generic rewrite of the proxy-mode section and folded this branch's two additions back into it. `hyp status` naming "every host the CA is permitted to vouch for" is still true and still this branch's change (LLP 0238 Consequences, extended but not superseded by 0262, and master's collectProxyTrust still returned no hosts). The launchd residue bullet is kept but re-scoped to `<client>` and to the attach that trusted the CA, since no attach path installs the variable now; the removal paths it documents (detach, --purge, uninstall, the #818 migration unwind) all still exist, as does the `launchd env:` status line. docs/PRIVACY.md, src/core/daemon/status.js and src/core/daemon/types.d.ts merged cleanly; the new proxy-trust `hosts` field does not collide with #818's `client_telemetry_stale` diagnostic or #777's status.json cleanup.
|
Deferred non-blocking review findings to #829. Triage at head 306210b. This head is the round-2 head 121f316 plus two merges of green master. The second merge (ecdfd82) resolved two conflicts with #818 (LLP 0262, Claude Code moved from proxy capture to OTEL telemetry attach): hypaware-core/plugins-workspace/claude/src/index.js resolved to master, deleting the attach dialog whose host line round 2 sanitized, because the dialog itself no longer exists; README.md folded this branch's two additions into master's client-generic proxy section. Verified here that the resolutions are sound: displayableCaHosts still guards the one remaining host-printing surface (src/core/daemon/status.js:1615), and the two readLocalCaInfo call sites #818 added in the gateway log only certPath and static strings, never certificate host bytes. Residual findings, all re-verified at this head and all non-blocking:
The deferred design items (F1 all-policy grant, F3 lifecycle residue, post-detach repair prompt) and the macOS acceptance items remain tracked in #829; the attach-dialog acceptance item is now moot and is noted there. Verified at this head: npm test 4495 pass / 0 fail, npm run typecheck clean, npm run smoke -- status_diagnostics ok. |
Fixes three of the findings on #793 and the doc omission it names. Every change was written and proven on Linux; the two macOS-only findings that need a real Mac (F1, F3 residue) are left open and called out at the bottom.
hyp statusdoes not name the CA's permitted hosts (medium) - fixedLLP 0238 Consequences: "The dialog and
hyp statusmust name all permitted hosts, so the grant is informed." The dialog half was implemented; the status half was not.collectProxyTrusthad the wholeLocalCaInfoin hand, includinghosts, and returned only the fingerprint.ProxyTrustReportgainshosts: string[], filled from the certificate's own permitteddNSNamesubtrees, so the line is the grant itself and cannot drift from config.permitted: api.anthropic.com, api.openai.com, chatgpt.cominside the existingproxy trust:block;--jsongainspermitted_hosts.dNSNameconstraint at all renders as "not host-limited" rather than as a blank line, since that reading is the one that matters.Proof (
test/core/status-proxy-trust.test.js): the new testevery permitted host the trust grant covers is named on both surfacesmints a CA over the full static provider set on an install that captures one of them, and asserts all three hosts on both surfaces. Before the fix:not ok 1,not ok 2(# pass 5 # fail 2). After:# pass 7 # fail 0.removeCaTrustdeletes one certificate per call (F2, low) - fixedEvery HypAware CA carries the same CN, so a re-minted machine holds several indistinguishable trusted roots and uninstall cleared exactly one. The survivors outlive the key they were minted with and nothing looks for them again.
securityreports no match left, which is also why "could not be found" already had to read as the end state rather than a failure.purgeProxyTrustResiduenow prints adetailalongside a successful removal: "some went" and "something is left" are no longer exclusive.Proof (
test/core/tls-darwin-trust.test.js):removeCaTrust clears every identically named root, not just the firstdrives a fake keychain holding three identically named certificates and asserts none survives. Two further tests cover the failure stop and the bound. Before the fix:not ok 6,not ok 7,not ok 8(# pass 6 # fail 3) - the three-duplicate keychain still held two roots. After:# pass 9 # fail 0.Caveat carried from the issue:
security's multi-match semantics are still unverified on a real Mac. The loop is safe under either reading - if one invocation already deleted all matches, the sweep costs exactly one extra call that reports not-found - but adocs/ACCEPTANCE.mdpass on a Mac with a re-minted CA is the confirmation.No timeout on the trust probes (low, pre-existing) - fixed
runServiceCommandspawned with no timeout, andhyp statuson darwin now shells out through it twice. The residual risk named on the issue is a locked login keychain raising a GUI prompt.runServiceCommand(bin, args, { timeoutMs })- opt-in, because the commands reaching this seam are not alike: a mutation the user is answering a password dialog for may take minutes, a read-only status probe may not. On expiry the child is SIGKILLed and the promise rejects with a newServiceCommandTimeoutError.isCaTrustedmaps exit codes to a boolean, so a killed process would otherwise render as "not trusted - runhyp attach claude", a false negative dressed as a measurement. As a rejection it lands on the existing catch and rendersunknown - the keychain probe could not run.isCaTrustedandisLaunchdEnvSetaccepttimeoutMs;hyp statuspasses 5s. The interactive attach path is untouched and still waits unbounded.Proof (
test/core/service-manager-test-sandbox.test.js):runServiceCommand kills a command that outlives its timeoutruns a 120s child withtimeoutMs: 250in a child process (the guard's opt-in is process-global, following the existing opt-in test's pattern). Before the fix:not ok 7(the helper takes no options, so the wait never ends and the outer 20s bound trips). After:ok 7. A companion test pins that an unset timeout still waits.Docs omit the launchd residue (low, omission) - fixed, non-behavioural
Neither
README.mdnordocs/PRIVACY.mdmentioned that macOS proxy attach setsNODE_USE_SYSTEM_CA=1vialaunchctl setenvand installs a login LaunchAgent to re-apply it. Both now state it, with the plist path, that it is a login item, that a running terminal app must be fully quit to see the variable, and that detach /--purge/ uninstall remove it. Every claim is read offsrc/core/daemon/launchd_env.js(installLaunchdEnv,removeLaunchdEnv,envAgentPlistPathunderdefaultPlistDir),src/core/config/client_detach_disk.js(releaseProxyModeLaunchdEnv) andsrc/core/commands/clients.js(purgeProxyTrustResidue). Both files also now mention thathyp statusnames the permitted hosts, which is true as of this PR.Still open
-p ssltouches the exact keychain-merge path the live macOS runs proved, and whether Bun's merge honours a policy-scoped trust setting is unverified. Needs a Mac and a Remote Control acceptance re-run, so it is not attempted here.launchd env:line on "a proxy attach is live" needsmodeplumbed out of the attach probe intoClientAttachReportplus a decision about whathyp statusclaims in that state (the same gate is exactly right for "CA minted by the daemon, never attached"). A design change; left for a doc.Checks
npm test: 4222 pass, 0 fail.npm run typecheck: clean.npm run smoke -- status_diagnosticsandnpm run smoke -- client_attach_idempotent: ok.docs/ACCEPTANCE.mdpass on a real Mac before a release that ships them.Fixes #793