fix: harden install/init against destroying proxy/env config (#313) - #314
Merged
Conversation
0.2.0's connect timer capped time-to-first-byte and synthesized 504s during upstream slow periods, killing Claude Code sessions machine-wide (subswitch#42). 0.3.0 (ADR-010 relay hardening) removes the TTFB cap and other proxy-side policy. Relay on this machine already restarted onto 0.3.0.
…#313) installSettings now uses a merge strategy (D-SETTINGS-1): hook entries are added idempotently to an existing settings.json by exact command string; statusLine and attribution are set only when absent; user-owned keys (env, permissions, model, apiKeyHelper) are never touched. Parse failure emits a warning and leaves the file byte-identical. Adds exported mergeDevflowSettingsTemplate helper for testing.
Adds proxyJsonExists() to proxy-state.ts — a discriminator helper that distinguishes "file absent" from readProxyState()'s tolerant Ok(defaultState) on ENOENT, which was indistinguishable from a file present with DEFAULT_PROXY_PORT. Gates ANTHROPIC_BASE_URL and CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT stripping in init.ts and uninstall.ts on proxyJsonExists() (D-STRIP-1): strip only when proxy.json exists, proving devflow previously wrote those env vars. Prevents destroying a user's independently-managed ANTHROPIC_BASE_URL during devflow init.
) In runProxyPreflight, the foreign ANTHROPIC_BASE_URL refusal (check ④) now runs BEFORE the adopted early-return (check ③). Previously, a healthy relay on the target port triggered an early-return that silently skipped the foreign-gateway guard entirely — enabling devflow's proxy while a user's custom gateway URL was set. D-EFR-5: readSettingsJson is called in the healthy-relay branch before returning Ok(adopted:true). swallowSettingsReadError semantics preserved: when that flag is true, readSettingsJson returns '{}' on I/O failure instead of throwing.
When the persisted binPath is missing or stale (e.g. npx cache GC cleared the
subswitch package dir), the ensure-proxy hook now attempts dependency-light
re-resolution before emitting the "relay binary not found" warning:
Strategy a: walk up from `command -v devflow` → find node_modules/subswitch →
read bin.subswitch from package.json via node (already present).
Strategy b: `command -v subswitch` — globally installed CLI.
Both strategies are best-effort; the hook falls through to the existing warning
and always exits 0 when re-resolution fails (avoids PF-001, PF-009). The healed
path is used for the current session only; `devflow proxy --enable` persists it.
Loop is bounded at 6 iterations (avoids PF-017).
packaging.test.ts hardcoded SUBSWITCH_VERSION='0.2.0'; the dependency was bumped to 0.3.0 in d087565. Align the guard constant with the current lockfile.
…313) - post-install.ts: hoist existingHooksObj and existing.hooks assignment out of the mergeDevflowSettingsTemplate loop — was re-evaluated on every iteration but always yielded the same reference after the first - ensure-proxy: remove "(moved here from below)" tombstone from node re-resolution comment; describe current state, not the transition
…313) The D-STRIP-1 gate was applied to the whole teardown instead of to the env strip alone, and it was missing entirely from `devflow proxy --disable`. - uninstall: with no proxy.json, `removeProxyHooks` stopped running too, so an uninstall could leave SessionStart/UserPromptSubmit entries pointing at a deleted hook script — and a re-run of an interrupted uninstall (proxy.json already removed by the artifact loop) could never clean them up. - `runDisable`: `applyDisableToSettings` still ran unconditionally with the DEFAULT_PROXY_PORT fallback, so `devflow proxy --disable` on a machine where Devflow never managed the proxy deleted a user's own ANTHROPIC_BASE_URL on 4141 plus CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT — the exact hazard #313 set out to fix. Both paths now route through `applyProxyTeardownToSettings(settings, port?)`: hooks always come out, the env strip runs only against a port the caller read from an existing proxy.json. `applyDisableToSettings` keeps its both-operations invariant and is still what runs on the managed path. Also folds the duplicated D-EFR-5 foreign-env block into a shared `checkSettingsEnv` helper, so the adopted-relay path and the free-port path apply the same check ④ — the adopted path was skipping the malformed-JSON refusal and the ANTHROPIC_API_KEY warning.
The subswitch 0.2.0 -> 0.3.0 bump on this branch retires two keys that were valid under 0.2.0 and are now registered legacy keys — a hard startup error, not a warning: `anthropic.streamIdleTimeoutMs` and `limits.maxConcurrentRequests`. `buildRoutingConfigJson` preserves a user's existing `anthropic` and `limits` blocks wholesale, so anyone who hand-tuned ~/.devflow/proxy-routing.json under 0.2.0 would have had those keys written straight back into the config the 0.3.0 relay reads — the relay refuses to boot, and the ensure-proxy hook just reports "relay failed to start" every session. Both keys join `limits.connectTimeoutMs` in a named ROUTING_CONFIG_REJECTED_SUBKEYS table so the two strip sites cannot drift. The list is scoped to keys a config that worked against the version Devflow actually shipped could contain — keys retired before 0.2.0 are unreachable. Also refreshes the @D-EFR-4 contract comment: in 0.3.0 anthropic.connectTimeoutMs is a genuine DNS+TCP connect budget (armed on the socket, disarmed on 'connect'), not the socket-inactivity timeout that motivated the 120s override under 0.2.0. The `preserves other anthropic fields` fixture used streamIdleTimeoutMs — a shape the pinned runtime now rejects — so it pinned behaviour that would break the relay at startup (avoids PF-043). Retargeted at maxUpstreamSockets.
…apes (#313) The confirm in `installSettings` was harmless while the old `hasHooks` check made it nearly unreachable. With the template merge it fires on every re-init for anyone with a feature turned off (HUD off -> no statusLine, memory off -> no memory hooks), and it fires from inside init's running spinner — the same place init deliberately calls `s.stop()` before prompting for sudo. Declining protected nothing either: init's own settings pass rewrites the whole hook set immediately afterwards. A prompt whose answer changes nothing is worse than no prompt, so the merge (additive only) now runs silently in both TTY and non-TTY. This also removes a `process.exit(0)` that would have fired under that spinner. `mergeDevflowSettingsTemplate` also walked a hand-editable file's shape unguarded: `hooks: []` silently swallowed the devflow entries (JSON.stringify drops keys attached to an array), and a matcher without a `hooks` array threw, which the outer catch turned into "could not configure settings" — hooks never installed. Every branch now validates shape at the sink and leaves foreign shapes untouched (applies PF-023). An empty `hooks` key is no longer introduced into a settings.json that had none.
…vior (#313) Add two tmpdir integration tests to tests/post-install-merge.test.ts: (a) parse-failure bail: write invalid JSON settings.json (trailing comma), call installSettings, assert file bytes identical and no .tmp.* residue. (b) merge wiring: write valid settings.json with env+permissions but no hooks, call installSettings, assert hooks are added and env/permissions survive byte-for-byte. Also correct a stale comment in src/cli/commands/uninstall.ts:1058 that still named applyDisableToSettings for a call that is now applyProxyTeardownToSettings. Co-Authored-By: Claude <noreply@anthropic.com>
Reflects proxy lifecycle changes from issue #313 / PR #314: proxyJsonExists() evidence discriminator, applyProxyTeardownToSettings unified teardown, D-STRIP-1 init.ts env-strip gating, checkSettingsEnv D-EFR-5 shared helper, ensure-proxy binPath re-resolution, post-install mergeDevflowSettingsTemplate merge strategy, and subswitch 0.3.0 routing config contract.
Streams over-window Anthropic-bound bodies instead of relay-synthesized 413, so long prompts no longer fail at the proxy; only translated (Codex) routes still return 413 request_too_large. limits.maxBodyBytes is renamed limits.maxBufferedBodyBytes, with the old spelling registered as a legacy key that makes the relay refuse to start. Includes the packaging version guard (SUBSWITCH_VERSION) so the pin and its assertion move together and the commit stays green under bisect. Refs: subswitch#42 / PR#43
buildRoutingConfigJson now strips limits.maxBodyBytes, renamed to
limits.maxBufferedBodyBytes in 0.4.0. The old spelling is a registered
legacy key that makes the relay refuse to start, so a hand-edited
proxy-routing.json carrying it would have killed the relay on every
session start — spawned by the ensure-proxy hook, with no route back.
This is the exact failure ROUTING_CONFIG_REJECTED_SUBKEYS exists to
prevent; it joins streamIdleTimeoutMs, limits.connectTimeoutMs, and
limits.maxConcurrentRequests.
DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS drops 120000 -> 10000. The 120s
override dates from 0.2.0, where the key was an upstream inactivity cap
whose 10s default killed long requests. Since 0.3.0 it is armed on the
socket and disarmed on connect, bounding only DNS+TCP, so the wide
budget bounded nothing extra and only delayed failure against an
unroutable host. 10000 matches the relay's own default; a user-supplied
value still wins. Pinned by literal value, not just by symbol — every
prior assertion compared against the imported constant and would have
stayed green if the value drifted.
Corrects the CLAUDE.md claim that proxy-routing.json is a bare {port}
object (it has always also carried anthropic.connectTimeoutMs) and
re-stamps 0.2.0-era catalog provenance comments, re-verified against the
0.4.0 binary: gpt-5.6-sol, gpt-5.6-terra, gpt-5.6-luna, gpt-5.5, and
models --json still exits 0 with configFileFound:false.
No devflow code depended on the removed 503 overloaded_error, the
concurrency admission gate, or the local 400 unknown-provider path.
…configs The relay's own default (10 s, DNS+TCP connect-only since subswitch 0.3.0) governs when the user has not set a value. Injection was a 0.2.0-era workaround artifact that outlived its purpose once the key's semantics were narrowed to connect-only — a wider budget bought nothing and only delayed failure against an unroutable host. User-set values are still preserved through the existing config-merge path.
Extend ROUTING_CONFIG_REJECTED_SUBKEYS.limits with the four 0.3.0-era limits.* keys that subswitch 0.4.0 promotes to hard startup errors: maxUpstreamSockets (→ anthropic.maxUpstreamSockets), streamIdleTimeoutMs (→ providers.codex.streamIdleTimeoutMs), requestTimeoutMs (→ providers.codex.requestTimeoutMs), and maxSseEventBytes (→ providers.codex.maxSseEventBytes). Also makes the limits block omit-when-empty (mirrors the existing anthropic branch), deletes the dangling bare ' *' JSDoc residue, and updates the proxy-log.ts provenance comment (three → four env vars, 0.2.0 → 0.4.0). Adds five new tests: one per new key (each asserting a neighbouring valid key survives) and one empty-after-strip test. Co-Authored-By: Claude <noreply@anthropic.com>
…CHANGELOG.md (#313) - CLAUDE.md: expand legacy-key strip list to all 7 limits.* sub-keys; replace stale "window var removed unconditionally" with the accurate evidence-gated strip description; add buildRoutingConfigJson port-only fresh-write note; document applyProxyTeardownToSettings, proxyJsonExists gating, foreign-gateway preflight, and binPath re-resolution. - KNOWLEDGE.md anti-pattern bullet: re-stamp from subswitch 0.3.0 to 0.4.0 and list all 7 stripped limits sub-keys. - index.md: keyword tail subswitch 0.3.0 → 0.4.0. - CHANGELOG.md: fold 4 additional strips into the existing Fixed entry; extend Upgrade note with the 7-key complete list. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
installSettingsnow merges devflow hook entries into an existingsettings.jsoninstead of replacing it wholesale. User-owned keys (env,permissions,model,apiKeyHelper, custom hooks) are never touched. Parse failure warns and skips — file left byte-identical. Idempotent by exact command string (D-SETTINGS-1). The interactive confirm prompt was removed entirely — the merge is additive/non-destructive, init's own settings pass rewrote hooks regardless of the answer anyway, and the prompt fired mid-spinner; the non-TTY "Settings exist without hooks" warning is gone too; foreign/unexpected hook shapes are left untouched (PF-023).applyProxyTeardownToSettings(the new exported helper that both uninstall and disable now route through) is unconditional, while env-var stripping remains evidence-gated on~/.devflow/proxy.jsonexistence viaproxyJsonExists().applyDisableToSettings's both-operations invariant (removeProxyHooks+_stripProxyEnvFromObject) is untouched — only the uninstall/disable wiring changed. PreviouslystripProxyEnvusedreadProxyState()'s tolerantOk(defaultState)on ENOENT, which caused stripping to run even when devflow had never managed the env vars (D-STRIP-1).buildRoutingConfigJsonnow strips sub-keys the 0.3.0 runtime hard-rejects on startup —anthropic.streamIdleTimeoutMs,limits.connectTimeoutMs, andlimits.maxConcurrentRequests— collected in the newROUTING_CONFIG_REJECTED_SUBKEYSconstant. The allowed top-key set is unchanged (port,logLevel,anthropic,providers,limits). The packaging test'sSUBSWITCH_VERSIONpin was updated to0.3.0.runProxyPreflight, the foreign-ANTHROPIC_BASE_URLrefusal (check ④) now evaluates before the adopted early-return (check ③). Previously, a healthy relay on the port triggered an early-return that silently skipped the foreign-gateway guard entirely (D-EFR-5).ensure-proxyshell hook now attempts dependency-light re-resolution ofbinPathbefore emitting the "relay binary not found" warning: walk fromcommand -v devflow→node_modules/subswitch/→ readbin.subswitchfrompackage.jsonvianode; fall back tocommand -v subswitch. Loop bounded at 6 iterations. Always exits 0.subswitch 0.3.0 → 0.4.0 Alignment
Three commits (69b963e → ba09de9 → c98bd06) advance the pinned runtime to
subswitch@0.4.0:chore(deps): bump subswitch 0.3.0 → 0.4.0. The 0.4.0 release streams over-window Anthropic-bound bodies upstream rather than rejecting them, eliminating the413 request_too_largefailures on long prompts. Codex routes still surface413per the upstream contract.fix(proxy): align subswitch integration with 0.4.0 config surface. Registerslimits.maxBodyBytesinROUTING_CONFIG_REJECTED_SUBKEYS(renamedlimits.maxBufferedBodyBytesin 0.4.0 — a registered legacy key that stops the relay from booting). UpdatesSUBSWITCH_VERSIONconstant intests/packaging.test.tsto0.4.0. Correctsanthropic.connectTimeoutMssemantics comment: the key is valid in 0.4.0 (strictly DNS+TCP connect budget, not an inactivity cap) and user-set values are preserved.fix(proxy): stop injecting anthropic.connectTimeoutMs into generated configs.buildRoutingConfigJsonno longer injectsanthropic.connectTimeoutMson a fresh write — the relay's own 10 s connect-only budget governs; the injection was a 0.2.0-era workaround whose semantics narrowed across versions. Fresh routing configs are now port-only.Follow-up (this PR's audit commits): Complete the
ROUTING_CONFIG_REJECTED_SUBKEYS.limitslist with the remaining four 0.4.0 LEGACY_KEY_ENTRIES —limits.maxUpstreamSockets(→anthropic.maxUpstreamSockets),limits.streamIdleTimeoutMs(→providers.codex.streamIdleTimeoutMs),limits.requestTimeoutMs(→providers.codex.requestTimeoutMs),limits.maxSseEventBytes(→providers.codex.maxSseEventBytes) — bringing the complete strip list to 7limits.*sub-keys. Thelimitsblock is now omitted when all sub-keys are stripped (mirrors the existinganthropicomit-when-empty behaviour). Tests: 5 new cases intests/proxy-state.test.ts(4 per-key + 1 empty-after-strip).Changes
src/targets/claude-code/post-install.ts— newmergeDevflowSettingsTemplate()exported helper;installSettings()rewritten to use merge strategy; interactive confirm prompt removedsrc/core/proxy-state.ts— newproxyJsonExists()exported helpersrc/cli/commands/proxy.ts— newapplyProxyTeardownToSettings()exported helper (unconditional hook removal + evidence-gated env strip); D-EFR-5: foreign-env check before adopted early-return inrunProxyPreflight;buildRoutingConfigJsonstripsROUTING_CONFIG_REJECTED_SUBKEYSfor 0.3.0 runtimesrc/cli/commands/init.ts— import + D-STRIP-1 gate forstripProxyEnvsrc/cli/commands/uninstall.ts— import + route throughapplyProxyTeardownToSettings(D-STRIP-1 gate); stale comment correctedsrc/assets/scripts/hooks/ensure-proxy— FIX 4 re-resolution block with bounded walktests/post-install-merge.test.ts— 22 unit tests formergeDevflowSettingsTemplate+ 2 tmpdir integration tests pinninginstallSettingsparse-failure and wiring (24 total)tests/proxy-state.test.ts— +8 tests forproxyJsonExists; +5 tests for 0.4.0 legacy-key strips and empty-after-striptests/proxy.test.ts— +11 tests (D-EFR-5 ordering REG-EFR-1/2/3 +ROUTING_CONFIG_REJECTED_SUBKEYSstripping +applyProxyTeardownToSettings)tests/shell-hooks.test.ts— +3 tests for FIX 4 re-resolutiontests/packaging.test.ts— bumpSUBSWITCH_VERSIONconstant to0.4.0Behavior Changes
settings.jsonexists: the interactive confirm asking whether to add Devflow hooks was removed. The merge is additive and non-destructive; declining could not protect any content the subsequent hooks-settings pass would not rewrite anyway. Non-TTY silent-skip path ("Settings exist without hooks" warning) is also removed.buildRoutingConfigJsonno longer injectsanthropic.connectTimeoutMs— the relay's own 10 s connect-only budget governs.Breaking Changes
None.
Reviewer Focus Areas
applyProxyTeardownToSettings:proxy.ts— unconditionalremoveProxyHookscall, then port-gated_stripProxyEnvFromObject; confirm theapplyDisableToSettingstwo-operation invariant remains intactinit.tsline ~1645 anduninstall.tsmanagedProxyPortsmap — both useproxyJsonExists()before touching env varsROUTING_CONFIG_REJECTED_SUBKEYS:proxy-state.ts— 7limits.*sub-keys +anthropic.streamIdleTimeoutMs; all are LEGACY_KEY_ENTRIES in 0.4.0; thelimitsblock is omitted when all are strippedproxy.tsrunProxyPreflight—readSettingsJson()is now called inside theisOurRelayBody()branch beforereturn Ok({ adopted: true })mergeDevflowSettingsTemplateuses exact command string matching; a second merge on an already-configured file produceschanged: falsewithout mutating anythingexit 0Related Issues
Closes #313