Stop offering a proxy migration and a CA repair the otel Claude attach cannot perform - #902
Conversation
neutral review round:
|
VerdictThe change is correct in its core claim: dropping Three findings, all in the new Findings1. medium -
|
Triage at the review-round cap (head a7e8a1d)The review-round budget is exhausted, so this head was triaged for residual findings. Outcome: no true blockers remain; the PR can merge safely. Deferred findings are tracked in #923. Verification performed at a7e8a1d
Residual findings, classified
No residual finding could cause wrong behaviour, data loss, a security hole, a crash, or a perf regression at this head. |
Triage re-run for head af988db (merge of master into fix/issue-885)The only new commit since the prior triage (a7e8a1d) is a merge of origin/master. Re-verified at this head:
Outcome: no true blockers; the PR can still merge safely. Deferred findings remain tracked in #923. |
philcunliffe
added
the
neutral:approved
…h cannot perform (#885) Two user-facing instructions left over from LLP 0262, where the `claude` client's attach became otel-only: one settings write, no repointed base URL, no CA, no keychain, no launchd env. 1. `hyp attach claude` on an install whose gateway config lacks `proxy_mode` offered the LLP 0244 migration, because `@hypaware/claude`'s picker row still declared `compose.gateway_proxy_mode`. Every clause of the question was false: the otel attach repoints no base URL, and a yes wrote `proxy_mode`, restarted the daemon, and blocked on a CA mint the following attach never consults. The scripted shapes carried the matching one-line pointer at a migration that does not exist. LLP 0243's own `Extended-by` line already settled the fix: the claude row stops declaring the flag, while the composition rule itself is unchanged. The `hyp init claude` preset, which writes literally what the fold composes, stops writing `proxy_mode: true` for the same reason: a claude-only install would mint a CA it never presents and then trip the gateway's own stale-CA warning. 2. `hyp status`'s proxy-trust block told a macOS host with a leftover CA that an untrusted keychain breaks Remote Control inbound and that `hyp attach claude` retries the trust, and that the same command sets `NODE_USE_SYSTEM_CA`. Nothing installs either any more (`installCaTrust` / `installLaunchdEnv` have no production callers since the otel switch), and Remote Control no longer rests on the trust at all, since the base URL is never repointed (LLP 0262 R6). Both lines now state the fact, and the block names the one command that acts on what it reports: `hyp detach claude --purge`. The migration machinery stays generic for any client still captured by the proxy, and keeps its coverage: the tests drive it through a staged installed plugin whose picker row declares the flag, instead of through `claude`. docs/PRIVACY.md carried the same stale claim ("attach installs the CA into your login keychain") and now describes it as residue of an earlier release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…stall to purge its CA Review of e1ea1a2 found the new `hyp status` proxy-trust note is unconditional, while `proxy_mode: true` still has the gateway mint and present that CA on every start (ai-gateway/src/source.js prepareInterception). On such an install the note read "only proxy_mode capture uses this CA, and the claude attach no longer does" and told the user to run `hyp detach claude --purge`, which deletes the key the running interception terminates TLS with; the next start mints a different one and nothing in the product restores the keychain trust. - ProxyTrustReport carries `proxyModeConfigured`, read off the effective gateway entry, because neither probe can tell a live CA from residue. - The note branches on it: residue keeps the purge, a live proxy_mode gateway is told what still depends on the CA and given nothing to run. `--json` gains `proxy_mode_configured` alongside. - The migration question no longer promises "macOS will then ask to trust the HypAware Local CA": `installCaTrust` has had no production caller since the attach went otel-only, so a yes mints the CA and stops there. - The preset comment claimed writing `proxy_mode: true` would leave the gateway logging its stale-CA warning; that warning only fires on the `!proxyMode` branch. Restated to what the key would actually cost. - PRIVACY.md and `hyp detach --help` no longer say a detach keeps the trust "so a later re-attach does not ask for your password again", which contradicts the same section's "nothing installs this CA into a system trust store any more". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…detach, or read an unreadable config as proxy_mode off
…ttach performs (#923) LLP 0262 moved Claude Code off proxy capture and took the last production caller of `installCaTrust` with it: nothing in the tree installs the interception CA into a trust store any more. The README's "Proxy mode" section was outside that change's diff and still described the removed step in the present tense, including the claim that a detach keeps the keychain trust "so re-attaching does not ask again". That last sentence is the one that costs the reader something. It reads the keychain trust an older release left on their account as a convenience being held for them, when it is a leftover nothing re-creates and only `hyp detach <client> --purge` or `hyp daemon uninstall` clears. The section now says trust is file-scoped to the proxied client's own settings, puts the keychain grant in the past tense as something earlier releases did, and tells a reader who ran one of those that the trust setting is still on their account until they remove it. `test/core/readme-proxy-trust-claims.test.js` is the gate: it pairs the fact that no production module outside `src/core/tls/darwin_trust.js` names `installCaTrust` with the claims the README may not make while that holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…es (#933 review) The fix for #923 corrected the CA-trust bullet and left the bullet under it making the same stale promise: "On macOS, a proxy attach also leaves a login-session variable behind." `installLaunchdEnv` has no production caller, so no attach runs `launchctl setenv NODE_USE_SYSTEM_CA 1` or writes the login agent. The bullet's own body had already drifted to past tense, so it contradicted its own lead. A reader turning proxy mode on for codex or claude-desktop was told to expect a session-wide variable and a login item that are never created. Also: "nothing installs it into a system trust store" understated the change, since the system keychain was never touched either way. What stopped is the login-keychain install, so the sentence now names it. The guard test only banned the three sentences this branch rewrote, so a paragraph re-documented from scratch could re-assert every one of them in fresh words and stay green. It now carries both halves: the banned claims, and the sentences the README has to keep saying instead. Its tree-fact assertion covers both trust writers, since the keychain root and the launchd variable that made the root count are one grant in two halves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The proxy-mode rewrite told README readers that no attach installs the interception CA into an OS trust store, and left `docs/PRIVACY.md` and `hyp detach --help` asserting the opposite in the present tense. The privacy document is where a reader checks what touched their machine and the help text is read at the moment of deciding, so those two were the worse copies to defer. - docs/PRIVACY.md: the keychain install, the password dialog, and the `launchctl setenv` step move to the past tense, and the lifetime paragraph stops selling a plain detach as holding the grant open for a re-attach that never asks again. - src/core/cli/core_commands.js: `hyp detach --help` says the kept trust is a leftover no attach re-creates, not a spared dialog. - README.md: `hyp detach <client>` does not unconditionally clear the launchd variable and its login agent. `releaseProxyModeLaunchdEnv` returns early unless the marker still records a proxy attach, so on a machine migrated to another attach mode (the machine the bullet is about) a plain detach is a no-op for the leftover. Only `--purge` and `hyp daemon uninstall` run the release unconditionally. The gate grows to match. Its claim lists are now per document, because the three say the same thing in three registers and the README's sentences read as non-sequiturs in a terminal help string; the old shared `REQUIRED_TRUTHS` was applied as a cross-product over `SCANNED`, so the widening the file invited would have demanded the README's verbatim prose of every document that joined. The installer scan is word-bounded so a future `installCaTrustForHost` is not read as this one, and its failure message says a module *names* a writer rather than accusing it of installing trust, which is all a text scan can prove. Renamed off `readme-` now that it covers three files.
af988db to
3d231b3
Compare
1 participant
Both findings in #885 reproduce, and both are fallout from #818 (LLP 0262), where the
claudeclient's attach became otel-only: one settings write, no repointed base URL, no CA, no keychain trust, no launchd env.1. The proxy-mode migration prompt
@hypaware/claude's picker row still declaredcompose.gateway_proxy_mode: true, which is the only gate onmaybeOfferProxyModeMigration. So a barehyp attach claudeon an install whose gateway config lacksproxy_modeasked:Every clause is false for an otel attach, and a yes wrote
proxy_mode, restarted the daemon, and waited on a CA mint the following attach never consults. The scripted shapes (--json, non-TTY,attach all) carried the matching pointer note at a migration that does not exist.Fix, as the corpus already settled it. LLP 0243 carries
Extended-by: LLP 0262 (... the Claude row stops declaring compose.gateway_proxy_mode ...; the composition rule itself is unchanged). The row drops the flag. The composition rule and the migration machinery stay generic for any client still captured by the proxy, and keep their coverage: the tests now drive them through a staged installed plugin whose picker row declares the flag.Two knock-ons of dropping the flag, both wanted:
proxy_mode: truefor a claude pick, so a fresh install mints no CA it will never present (LLP 0243's own "a Codex-only install mints no CA it will never use", now true of claude too);hyp init claudepreset, which writes literally what the fold composes, stops writing the key for the same reason. Left in, it would trip the gateway's own stale-CA warning on first boot.2.
hyp statusCA-trust advicedescribeCaTrust(false)claimed "Remote Control inbound will not work, runhyp attach claudeto retry" anddescribeLaunchdEnv(false)claimed "runhyp attach claudeto set it". Confirmed:installCaTrustandinstallLaunchdEnvhave no callers outsidetest/since #818, and Remote Control does not rest on the trust any more, because the base URL is never repointed (LLP 0262 R6). Both lines now state the fact, and the block names the one command that does act on what it reports:--jsonkeeps its existing keys and adds the tri-stateproxy_trust.proxy_mode_configuredfield.docs/PRIVACY.mdcarried the same stale claim ("attach installs the CA into your login keychain") and now describes it as residue of an earlier release.Reproduced first
Four assertions written against the current code, all failing before the fix:
test/core/attach-proxy-migration.test.js- "claude is never offered the proxy-mode switch: its attach is otel-only" (the question was asked) and "a scripted claude attach carries no base-URL pointer note" (the note was written).test/core/status-proxy-trust.test.js- the trusted and untrusted cases now assert nohyp attach claudeanywhere in the block, and the purge pointer instead.Checks
npm test: 4486 pass, 0 fail.npm run typecheck: clean.npm run smoke:claude_attach_detach,walkthrough_picker_to_first_query,client_attach_idempotent,client_attach_on_join,claude_telemetry_capture,status_diagnosticsall green (the first two carried the old expectations and are updated here).For the reviewer
With the claude row's declaration gone, no bundled picker row declares
gateway_proxy_mode, somaybeOfferProxyModeMigrationandenableGatewayProxyModeare dormant in a stock install. They are kept, not deleted: LLP 0262 keeps the gateway proxy fully supported for the clients that still route through it, LLP 0243 says the composition rule is unchanged, and retiring the machinery outright is a design call worth making deliberately rather than as fallout from a wording fix. Both remain under test.Fixes #885