fix: scope machine-only settings to application (#1032)#1034
Draft
EhabY wants to merge 7 commits into
Draft
Conversation
Change these two settings from "machine" to "application" scope, with ignoreSync: true so they still don't sync via Settings Sync. This keeps the SEC-200 goal (workspace/folder settings.json can't override them) while testing whether "machine" scope is responsible for reports in #1032 where the User-configured binaryDestination stops being honored on reconnect.
… settings Also move coder.headerCommand, coder.tlsCertFile, coder.tlsKeyFile, coder.tlsCaFile, coder.tlsAltHost, coder.tlsCertRefreshCommand, and coder.proxyLogDirectory from machine to application scope, with ignoreSync: true. These are all read at the same pre-connection point as coder.binaryDestination, so they share the same exposure to the machine-scope reconnect issue. Left coder.proxyBypass (deprecated in favor of http.noProxy), coder.sshFlags, coder.globalFlags, and coder.sshConfig (array-typed) out of this change for now.
Array-typed settings resolve through the same scope mechanism as string settings, so they carry the same exposure to the machine-scope reconnect issue. Confirmed via a minimal test extension that machine-scoped array settings resolve identically to string settings on Linux.
… settings - Flip coder.sshConfig and coder.proxyBypass from machine to application scope for the same reason as the other settings in this branch: both are read during pre-connection setup and are subject to the same VS Code local-configuration exclusion described in #1032/#1034. - Re-review ignoreSync per setting instead of blanket-applying it. Settings holding a local file path or an executed local command keep ignoreSync: true (coder.binaryDestination, coder.headerCommand, coder.tlsCertFile, coder.tlsKeyFile, coder.tlsCaFile, coder.tlsCertRefreshCommand, coder.proxyLogDirectory, coder.globalFlags, coder.sshConfig). Plain URLs/hostnames/flags with no local-path dependency now sync like other application-scoped settings (coder.binarySource, coder.tlsAltHost, coder.proxyBypass, coder.sshFlags). - Document the Settings Sync behavior in the README and CHANGELOG.
Revert the earlier attempt to let coder.binarySource, coder.tlsAltHost, coder.proxyBypass, and coder.sshFlags start syncing. Their values (a download URL, a TLS hostname override, a proxy no_proxy list, and CLI flags) can still be tied to a specific machine's network or environment even though they aren't filesystem paths, so opportunistically enabling sync for them isn't something this scope-resolution fix needs. All 13 settings moved off machine scope keep ignoreSync: true, matching their pre-existing behavior exactly; only the read-scope for local-vs-remote configuration changes.
Once a window has a remote connection, VS Code stops reading machine-scoped values from the local settings.json at all, with or without Profiles (hasRemote is the trigger, not profile usage). Add a README warning pointing back to #1032/#1034 so no one reintroduces machine scope without addressing this first.
We only confirmed the mechanism from VS Code source and real user reports, not from our own reproduction attempts (5/5 negative, including a full real-SSH reconnect where hasRemote was unambiguously true). Say so plainly instead of implying we validated the trigger ourselves.
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
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
Changes these settings from
"scope": "machine"to"scope": "application"(all keepignoreSync: true, unchanged from before, so Settings Sync behavior for them doesn't change):coder.binarySourcecoder.binaryDestinationcoder.headerCommandcoder.tlsCertFilecoder.tlsKeyFilecoder.tlsCaFilecoder.tlsAltHostcoder.tlsCertRefreshCommandcoder.proxyLogDirectorycoder.proxyBypass(deprecated in favor ofhttp.noProxy)coder.sshConfigcoder.sshFlagscoder.globalFlagsWe considered letting a few of these (plain URL/hostname/flag settings with no filesystem path) start syncing now that they're
application-scoped, but held off: values like a custom download URL, a TLS hostname override, a proxyno_proxylist, or CLI flags can still be tied to a specific machine's network or environment even without being a literal path, and none of them synced before. That's a separate, deliberate change with its own discussion, not something this scope-resolution fix needs. So every setting here keeps its exact pre-existing sync behavior; only the local-vs-remote read scope changes.Why
machinescope was introduced in #965 (SEC-200) to stop workspace and foldersettings.jsonfrom overriding these settings. That security goal still needs to hold, andapplicationscope keeps it: it can only be set in User settings, never overridden by workspace or folder settings.The reporters in #1032 confirmed that
machinescope also causes VS Code to stop honoring the User-configured value when reconnecting to a previously connected remote workspace. First connection works, reconnecting fails, even though nothing in the settings changed. Two independent users confirmed a test build withapplicationscope fixes this on Windows 11, and pin the regression to v1.14.6, the exact release that shipped #965.All of these settings are read at the same point during the extension's pre-connection setup, right before the SSH connection is established, so they share the same exposure. Array-typed settings (
sshFlags/globalFlags) carry the same risk as the string ones: VS Code's scope resolution operates on the setting key regardless of value type (confirmed with a minimal test extension).The VS Code mechanism
VS Code's
WorkspaceServicebuilds a window's local user configuration with a scope filter (getLocalUserConfigurationScopesinconfigurationService.ts). Once a window has a remote authority,machine-scoped keys are dropped from that filter (LOCAL_MACHINE_SCOPESexcludesMACHINE; seeconfiguration.ts). Amachine-scoped value can then only come from the remote machine's settings file, not the local one where the user actually configured it.This is deliberate, long-standing VS Code design, not a recent VS Code change:
http.proxy(amachine-scoped setting) before/during connection setup, and fixed it the same way we are here: "Made proxy settings application scope by default. Once connected to remote, scope is changed accordingly."configurationService.test.ts, suiteWorkspaceConfigurationService - Remote Folder, testmachine settings in local user settings does not override defaults. It writes amachine-scoped key to the local usersettings.json, constructsWorkspaceServicewith aremoteAuthority, and assertsgetValue()returns the default, not the configured value.We verified this ourselves by building VS Code from source and adding a test using our exact setting: writing
{"coder.binaryDestination": "/usr/bin/coder"}to the local usersettings.json, constructingWorkspaceServicewith aremoteAuthority, and confirminginspect('coder.binaryDestination').userLocalValuecorrectly shows/usr/bin/coder(the value IS in the file) whilegetValue('coder.binaryDestination')returns''(the default) instead. This test passed on a single fresh connection, no reconnect needed, matching the official suite. So theWorkspaceService-level mechanism is confirmed directly against our own setting, not just VS Code's generic fixture.So what changed for us wasn't VS Code: it's that v1.14.6 (#965) started using
machinescope for these settings for the first time, which activated this pre-existing filtering behavior for settings that had never been subject to it before.This PR and investigation were assisted by Coder Agents on behalf of @EhabY.