Skip to content

Commit a8bff3e

Browse files
fioan89fioan89
authored
refactor: remove unsafe non-null assertions to prevent race condition (#205)
Replace !! operators with safe idiom takeIf/let chains. The non-null assertions were unsafe in concurrent scenarios where one thread could potentially modify the settings while another thread reads and makes non-null assertions.
1 parent 404efce commit a8bff3e

File tree

6 files changed

+64
-52
lines changed

6 files changed

+64
-52
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Fixed
6+
7+
- potential race condition that could cause crashes when settings are modified concurrently
8+
59
## 0.7.0 - 2025-09-27
610

711
### Changed

‎src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -354,24 +354,22 @@ class CoderCLIManager(
354354
// always use the correct URL.
355355
"--url",
356356
escape(deploymentURL.toString()),
357-
if (!context.settingsStore.headerCommand.isNullOrBlank()) "--header-command" else null,
358-
if (!context.settingsStore.headerCommand.isNullOrBlank()) escapeSubcommand(context.settingsStore.headerCommand!!) else null,
357+
context.settingsStore.headerCommand?.takeIf { it.isNotBlank() }?.let { "--header-command" },
358+
context.settingsStore.headerCommand?.takeIf { it.isNotBlank() }?.let { escapeSubcommand(it) },
359359
"ssh",
360360
"--stdio",
361361
if (context.settingsStore.disableAutostart && feats.disableAutostart) "--disable-autostart" else null,
362362
"--network-info-dir ${escape(context.settingsStore.networkInfoDir)}"
363363
)
364364
val proxyArgs = baseArgs + listOfNotNull(
365-
if (!context.settingsStore.sshLogDirectory.isNullOrBlank()) "--log-dir" else null,
366-
if (!context.settingsStore.sshLogDirectory.isNullOrBlank()) escape(context.settingsStore.sshLogDirectory!!) else null,
365+
context.settingsStore.sshLogDirectory?.takeIf { it.isNotBlank() }?.let { "--log-dir" },
366+
context.settingsStore.sshLogDirectory?.takeIf { it.isNotBlank() }?.let { escape(it) },
367367
if (feats.reportWorkspaceUsage) "--usage-app=jetbrains" else null,
368368
)
369-
val extraConfig =
370-
if (!context.settingsStore.sshConfigOptions.isNullOrBlank()) {
371-
"\n" + context.settingsStore.sshConfigOptions!!.prependIndent(" ")
372-
} else {
373-
""
374-
}
369+
val extraConfig = context.settingsStore.sshConfigOptions
370+
?.takeIf { it.isNotBlank() }
371+
?.let { "\n" + it.prependIndent(" ") }
372+
?: ""
375373
val options = """
376374
ConnectTimeout 0
377375
StrictHostKeyChecking no

‎src/main/kotlin/com/coder/toolbox/sdk/CoderHttpClientBuilder.kt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ object CoderHttpClientBuilder {
2121
val trustManagers = coderTrustManagers(settings.tls.caPath)
2222
var builder = OkHttpClient.Builder()
2323

24-
if (context.proxySettings.getProxy() != null) {
25-
context.logger.info("proxy: ${context.proxySettings.getProxy()}")
26-
builder.proxy(context.proxySettings.getProxy())
27-
} else if (context.proxySettings.getProxySelector() != null) {
28-
context.logger.info("proxy selector: ${context.proxySettings.getProxySelector()}")
29-
builder.proxySelector(context.proxySettings.getProxySelector()!!)
24+
context.proxySettings.getProxy()?.let { proxy ->
25+
context.logger.info("proxy: $proxy")
26+
builder.proxy(proxy)
27+
} ?: context.proxySettings.getProxySelector()?.let { proxySelector ->
28+
context.logger.info("proxy selector: $proxySelector")
29+
builder.proxySelector(proxySelector)
3030
}
3131

3232
// Note: This handles only HTTP/HTTPS proxy authentication.

‎src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt‎

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,9 @@ import com.coder.toolbox.sdk.v2.models.CreateWorkspaceBuildRequest
1515
import com.coder.toolbox.sdk.v2.models.Template
1616
import com.coder.toolbox.sdk.v2.models.User
1717
import com.coder.toolbox.sdk.v2.models.Workspace
18-
import com.coder.toolbox.sdk.v2.models.WorkspaceAgent
1918
import com.coder.toolbox.sdk.v2.models.WorkspaceBuild
2019
import com.coder.toolbox.sdk.v2.models.WorkspaceBuildReason
2120
import com.coder.toolbox.sdk.v2.models.WorkspaceResource
22-
import com.coder.toolbox.sdk.v2.models.WorkspaceStatus
2321
import com.coder.toolbox.sdk.v2.models.WorkspaceTransition
2422
import com.squareup.moshi.Moshi
2523
import okhttp3.OkHttpClient
@@ -114,7 +112,9 @@ open class CoderRestClient(
114112
)
115113
}
116114

117-
return userResponse.body()!!
115+
return requireNotNull(userResponse.body()) {
116+
"Successful response returned null body or user"
117+
}
118118
}
119119

120120
/**
@@ -132,41 +132,29 @@ open class CoderRestClient(
132132
)
133133
}
134134

135-
return workspacesResponse.body()!!.workspaces
135+
return requireNotNull(workspacesResponse.body()?.workspaces) {
136+
"Successful response returned null body or workspaces"
137+
}
136138
}
137139

138140
/**
139141
* Retrieves a workspace with the provided id.
140142
* @throws [APIResponseException].
141143
*/
142144
suspend fun workspace(workspaceID: UUID): Workspace {
143-
val workspacesResponse = retroRestClient.workspace(workspaceID)
144-
if (!workspacesResponse.isSuccessful) {
145+
val workspaceResponse = retroRestClient.workspace(workspaceID)
146+
if (!workspaceResponse.isSuccessful) {
145147
throw APIResponseException(
146148
"retrieve workspace",
147149
url,
148-
workspacesResponse.code(),
149-
workspacesResponse.parseErrorBody(moshi)
150+
workspaceResponse.code(),
151+
workspaceResponse.parseErrorBody(moshi)
150152
)
151153
}
152154

153-
return workspacesResponse.body()!!
154-
}
155-
156-
/**
157-
* Maps the available workspaces to the associated agents.
158-
*/
159-
suspend fun workspacesByAgents(): Set<Pair<Workspace, WorkspaceAgent>> {
160-
// It is possible for there to be resources with duplicate names so we
161-
// need to use a set.
162-
return workspaces().flatMap { ws ->
163-
when (ws.latestBuild.status) {
164-
WorkspaceStatus.RUNNING -> ws.latestBuild.resources
165-
else -> resources(ws)
166-
}.filter { it.agents != null }.flatMap { it.agents!! }.map {
167-
ws to it
168-
}
169-
}.toSet()
155+
return requireNotNull(workspaceResponse.body()) {
156+
"Successful response returned null body or workspace"
157+
}
170158
}
171159

172160
/**
@@ -187,7 +175,10 @@ open class CoderRestClient(
187175
resourcesResponse.parseErrorBody(moshi)
188176
)
189177
}
190-
return resourcesResponse.body()!!
178+
179+
return requireNotNull(resourcesResponse.body()) {
180+
"Successful response returned null body or workspace resources"
181+
}
191182
}
192183

193184
suspend fun buildInfo(): BuildInfo {
@@ -200,7 +191,10 @@ open class CoderRestClient(
200191
buildInfoResponse.parseErrorBody(moshi)
201192
)
202193
}
203-
return buildInfoResponse.body()!!
194+
195+
return requireNotNull(buildInfoResponse.body()) {
196+
"Successful response returned null body or build info"
197+
}
204198
}
205199

206200
/**
@@ -216,7 +210,10 @@ open class CoderRestClient(
216210
templateResponse.parseErrorBody(moshi)
217211
)
218212
}
219-
return templateResponse.body()!!
213+
214+
return requireNotNull(templateResponse.body()) {
215+
"Successful response returned null body or template"
216+
}
220217
}
221218

222219
/**
@@ -238,7 +235,10 @@ open class CoderRestClient(
238235
buildResponse.parseErrorBody(moshi)
239236
)
240237
}
241-
return buildResponse.body()!!
238+
239+
return requireNotNull(buildResponse.body()) {
240+
"Successful response returned null body or workspace build"
241+
}
242242
}
243243

244244
/**
@@ -254,7 +254,10 @@ open class CoderRestClient(
254254
buildResponse.parseErrorBody(moshi)
255255
)
256256
}
257-
return buildResponse.body()!!
257+
258+
return requireNotNull(buildResponse.body()) {
259+
"Successful response returned null body or workspace build"
260+
}
258261
}
259262

260263
/**
@@ -296,7 +299,10 @@ open class CoderRestClient(
296299
buildResponse.parseErrorBody(moshi)
297300
)
298301
}
299-
return buildResponse.body()!!
302+
303+
return requireNotNull(buildResponse.body()) {
304+
"Successful response returned null body or workspace build"
305+
}
300306
}
301307

302308
fun close() {

‎src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,10 @@ open class CoderProtocolHandler(
252252
parameters: Map<String, String?>,
253253
workspace: Workspace,
254254
): WorkspaceAgent? {
255-
val agents = workspace.latestBuild.resources.filter { it.agents != null }.flatMap { it.agents!! }
255+
val agents = workspace.latestBuild.resources
256+
.mapNotNull { it.agents }
257+
.flatten()
258+
256259
if (agents.isEmpty()) {
257260
context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "The workspace \"${workspace.name}\" has no agents")
258261
return null

‎src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,16 @@ class ConnectStep(
5656
return
5757
}
5858

59-
statusField.textState.update { context.i18n.pnotr("Connecting to ${CoderCliSetupContext.url!!.host}...") }
59+
statusField.textState.update { context.i18n.pnotr("Connecting to ${CoderCliSetupContext.url?.host ?: "unknown host"}...") }
6060
connect()
6161
}
6262

6363
/**
6464
* Try connecting to Coder with the provided URL and token.
6565
*/
6666
private fun connect() {
67-
if (!CoderCliSetupContext.hasUrl()) {
67+
val url = CoderCliSetupContext.url
68+
if (url == null) {
6869
errorField.textState.update { context.i18n.ptrl("URL is required") }
6970
return
7071
}
@@ -74,15 +75,15 @@ class ConnectStep(
7475
return
7576
}
7677
// Capture the host name early for error reporting
77-
val hostName = CoderCliSetupContext.url!!.host
78+
val hostName = url.host
7879

7980
signInJob?.cancel()
8081
signInJob = context.cs.launch(CoroutineName("Http and CLI Setup")) {
8182
try {
8283
context.logger.info("Setting up the HTTP client...")
8384
val client = CoderRestClient(
8485
context,
85-
CoderCliSetupContext.url!!,
86+
url,
8687
if (context.settingsStore.requireTokenAuth) CoderCliSetupContext.token else null,
8788
PluginManager.pluginInfo.version,
8889
)

0 commit comments

Comments
 (0)