Add option to disable downloading Coder binary even if out of date or missing

[jfhovinne]

• Option to disable downloading
• If the binary is read-only, automatically skip downloading, maybe output a warning < Decided not to do this at least for now in favor of being explicitly and predictably controlled by settings
• Check the version beforehand in case etags are not supported

In some environments, it is not allowed to download then execute arbitrary binaries; such binaries have to be approved, specifically packaged and then installed from a central application repository through e.g. UI tooling.

#122 adds settings for the Coder binary source and destination; would it be possible to also allow pointing to an existing coder executable (e.g. "C:\Program Files\Coder\bin\coder-windows-amd64.exe"), and disable the remote download?

[code-asher]

Yeah we should add an option to disable downloading like our JetBrains plugin has.

That said, the VS Code plugin should only try to download if the binary is the wrong version, so setting binaryDestination to C:\Program Files\Coder\bin\ should work as long as it is the right version. Is that not the behavior you are seeing?

[jfhovinne]

@code-asher No, as I understand it from my testing and checking storage.fetchBinary(), the extension downloads the binary and tries to save it in binaryDestination (which is read-only), then tries to replace the existing one. This leads to the below error, which might be confusing for the end-user:

Error: ENOENT: no such file or directory, rename 'C:\Program Files\Coder\bin\coder-windows-amd64.exe.temp-*****' -> 'C:\Program Files\Coder\bin\coder-windows-amd64.exe'

The extension seems usable afterwards though.

[code-asher]

Huh, that is strange, assuming it is the same version (the hash matches, sent via the If-None-Match header) the request should result in a 304.

vscode-coder/src/storage.ts

Lines 226 to 229 in ae17065

	case 304: {
	this.output.appendLine(`Using cached binary: ${binPath}`)
	return binPath
	}

I suppose this implies that the endpoint is returning a 200.

vscode-coder/src/storage.ts

Lines 214 to 216 in ae17065

	await fs.rename(binPath, oldBinPath).catch(() => {
	this.output.appendLine(`Warning: failed to rename ${binPath} to ${oldBinPath}`)
	})

Is the binary being downloaded from somewhere that is not supporting the If-None-Match header, maybe?

In the JetBrains plugin one thing we do is check the version before making the request with coder version --json in case the source does not support etags, so we might want to do the same here as well.

[code-asher]

And good point about the error message, we should change that to something like "Failed to download binary, continuing anyway" or something like that. Maybe we should only try to continue if there is an existing binary there, outdated or not.

[jfhovinne]

Thanks for the pointers; indeed, could also be an issue with the forward or reverse proxies removing the header; to be investigated.
Anyway, since the extension will not be able to update the binary - in any case - failing gracefully as you suggest would at least improve UX.
But I guess being able to disable the download like in the JetBrains plugin as you describe would definitely help.

[michaelbrewer]

Yeah, this is a blocker for us to. @kylecarbs

[kylecarbs]

Will fix @michaelbrewer

[code-asher]

@michaelbrewer Is binaryDestination not working? It should use the binary in there and will only try to download if it is the wrong version, if I recall correctly.

I will update the issue title too.

[michaelbrewer]

@code-asher turns out the bigger issue is the coder binary not being signed.