Skip to content

Commit 0a73918

Browse files
ctronclaude
ctron
and
claude
committed
feat: add file-system locking for concurrent config access
Wrap all config read-modify-write cycles in Config::locked(), which acquires an advisory exclusive lock on a sidecar .lock file. This prevents race conditions when the MCP server and CLI commands run concurrently. Uses std::fs::File::lock() (stable since 1.89) with spawn_blocking and AsyncFnOnce closures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 87d3a67 commit 0a73918

8 files changed

Lines changed: 220 additions & 192 deletions

File tree

‎.github/workflows/ci.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ jobs:
3232

3333
rust:
3434
- stable
35-
- "1.88" # MSRV
35+
- "1.89" # MSRV
3636

3737
os:
3838
- ubuntu-latest

‎Cargo.toml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ repository = "https://github.com/ctron/oidc-cli"
1010
categories = ["command-line-utilities", "authentication"]
1111
keywords = ["oidc", "cli"]
1212
readme = "README.md"
13-
# based on comfy table, requiring more recent lang features
14-
rust-version = "1.88"
13+
# File::lock() requires 1.89
14+
rust-version = "1.89"
1515

1616
[[bin]]
1717
name = "oidc"

‎src/cmd/create/confidential.rs‎

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -33,48 +33,47 @@ impl CreateConfidential {
3333
pub async fn run(self) -> anyhow::Result<()> {
3434
log::debug!("creating new client: {}", self.common.name);
3535

36-
let mut config = Config::load(self.config.as_deref())?;
37-
38-
if !self.common.force && config.clients.contains_key(&self.common.name) {
39-
bail!(
40-
"A client named '{}' already exists. You need to delete it first or use --force",
41-
self.common.name
42-
);
43-
}
44-
45-
let mut client = Client {
46-
issuer_url: self.common.issuer,
47-
scope: self.common.scope,
48-
r#type: ClientType::Confidential {
49-
client_id: self.client_id,
50-
client_secret: self.client_secret,
51-
},
52-
state: None,
53-
};
54-
55-
if !self.common.skip_initial {
56-
let token = get_token(&client, &self.http)
57-
.await
58-
.context("failed retrieving first token")?;
59-
60-
let token = match token {
61-
TokenResult::Refreshed(token) | TokenResult::Existing(token) => token,
36+
Config::locked(self.config.as_deref(), async |config| {
37+
if !self.common.force && config.clients.contains_key(&self.common.name) {
38+
bail!(
39+
"A client named '{}' already exists. You need to delete it first or use --force",
40+
self.common.name
41+
);
42+
}
43+
44+
let mut client = Client {
45+
issuer_url: self.common.issuer.clone(),
46+
scope: self.common.scope.clone(),
47+
r#type: ClientType::Confidential {
48+
client_id: self.client_id.clone(),
49+
client_secret: self.client_secret.clone(),
50+
},
51+
state: None,
6252
};
6353

64-
log::info!("First token:");
65-
log::info!(" ID: {}", OrNone(&token.id_token));
66-
log::info!(" Access: {}", token.access_token);
67-
log::info!(" Refresh: {}", OrNone(&token.refresh_token));
54+
if !self.common.skip_initial {
55+
let token = get_token(&client, &self.http)
56+
.await
57+
.context("failed retrieving first token")?;
58+
59+
let token = match token {
60+
TokenResult::Refreshed(token) | TokenResult::Existing(token) => token,
61+
};
6862

69-
client.state = Some(token);
70-
}
63+
log::info!("First token:");
64+
log::info!(" ID: {}", OrNone(&token.id_token));
65+
log::info!(" Access: {}", token.access_token);
66+
log::info!(" Refresh: {}", OrNone(&token.refresh_token));
7167

72-
config
73-
.clients
74-
.insert(self.common.name.clone(), client.clone());
68+
client.state = Some(token);
69+
}
7570

76-
config.store(self.config.as_deref())?;
71+
config
72+
.clients
73+
.insert(self.common.name.clone(), client.clone());
7774

78-
Ok(())
75+
Ok(())
76+
})
77+
.await
7978
}
8079
}

‎src/cmd/create/public.rs‎

Lines changed: 60 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -75,72 +75,72 @@ impl CreatePublic {
7575
pub async fn run(self) -> anyhow::Result<()> {
7676
log::debug!("creating new client: {}", self.common.name);
7777

78-
let mut config = Config::load(self.config.as_deref())?;
79-
80-
if !self.common.force && config.clients.contains_key(&self.common.name) {
81-
bail!(
82-
"A client named '{}' already exists. You need to delete it first or use --force",
83-
self.common.name
84-
);
85-
}
86-
87-
let http = create_client(&self.http).await?;
88-
89-
let provider_metadata =
90-
CoreProviderMetadata::discover_async(self.common.issuer.clone(), &http).await?;
91-
92-
let client = CoreClient::from_provider_metadata(
93-
provider_metadata,
94-
ClientId::new(self.client_id.clone()),
95-
self.client_secret.clone().map(ClientSecret::new),
96-
);
97-
98-
let token = match self.refresh_token {
99-
None => self.code_flow(&http, &client).await?,
100-
Some(refresh_token) => {
101-
refresh_token_request(&http, &client, self.common.scope.as_deref(), refresh_token)
102-
.await?
78+
Config::locked(self.config.as_deref(), async |config| {
79+
if !self.common.force && config.clients.contains_key(&self.common.name) {
80+
bail!(
81+
"A client named '{}' already exists. You need to delete it first or use --force",
82+
self.common.name
83+
);
10384
}
104-
};
105-
106-
// log info
107-
108-
log::info!("First token:");
109-
log::info!(
110-
" ID: {}",
111-
OrNone(
112-
&token
113-
.extra_fields()
114-
.id_token()
115-
.cloned()
116-
.map(|t| t.to_string())
117-
)
118-
);
119-
log::info!(" Access: {}", token.access_token().clone().into_secret());
120-
log::info!(
121-
" Refresh: {}",
122-
OrNone(&token.refresh_token().cloned().map(|t| t.into_secret()))
123-
);
12485

125-
// create client
86+
let http = create_client(&self.http).await?;
12687

127-
let client = Client {
128-
issuer_url: self.common.issuer,
129-
scope: self.common.scope,
130-
r#type: ClientType::Public {
131-
client_id: self.client_id,
132-
client_secret: self.client_secret,
133-
},
134-
state: Some(token.into()),
135-
};
88+
let provider_metadata =
89+
CoreProviderMetadata::discover_async(self.common.issuer.clone(), &http).await?;
13690

137-
config
138-
.clients
139-
.insert(self.common.name.clone(), client.clone());
91+
let client = CoreClient::from_provider_metadata(
92+
provider_metadata,
93+
ClientId::new(self.client_id.clone()),
94+
self.client_secret.clone().map(ClientSecret::new),
95+
);
14096

141-
config.store(self.config.as_deref())?;
97+
let token = match &self.refresh_token {
98+
None => self.code_flow(&http, &client).await?,
99+
Some(refresh_token) => {
100+
refresh_token_request(
101+
&http,
102+
&client,
103+
self.common.scope.as_deref(),
104+
refresh_token.clone(),
105+
)
106+
.await?
107+
}
108+
};
109+
110+
log::info!("First token:");
111+
log::info!(
112+
" ID: {}",
113+
OrNone(
114+
&token
115+
.extra_fields()
116+
.id_token()
117+
.cloned()
118+
.map(|t| t.to_string())
119+
)
120+
);
121+
log::info!(" Access: {}", token.access_token().clone().into_secret());
122+
log::info!(
123+
" Refresh: {}",
124+
OrNone(&token.refresh_token().cloned().map(|t| t.into_secret()))
125+
);
142126

143-
Ok(())
127+
let client = Client {
128+
issuer_url: self.common.issuer.clone(),
129+
scope: self.common.scope.clone(),
130+
r#type: ClientType::Public {
131+
client_id: self.client_id.clone(),
132+
client_secret: self.client_secret.clone(),
133+
},
134+
state: Some(token.into()),
135+
};
136+
137+
config
138+
.clients
139+
.insert(self.common.name.clone(), client.clone());
140+
141+
Ok(())
142+
})
143+
.await
144144
}
145145

146146
fn bind_mode(&self) -> Bind {
@@ -191,12 +191,8 @@ Open the following URL in your browser and perform the interactive login process
191191
);
192192
}
193193

194-
// receive the result from the local server
195-
196194
let result = server.receive_token().await?;
197195

198-
// validate CSRF token
199-
200196
match result.state {
201197
None => {
202198
bail!("missing 'state' parameter from server");
@@ -207,16 +203,12 @@ Open the following URL in your browser and perform the interactive login process
207203
Some(_) => {}
208204
}
209205

210-
// fetch token
211-
212206
let token = client
213207
.exchange_code(AuthorizationCode::new(result.code))?
214208
.set_pkce_verifier(pkce_verifier)
215209
.request_async(http)
216210
.await?;
217211

218-
// check ID token
219-
220212
if let Some(id_token) = token.extra_fields().id_token() {
221213
let scopes = self.common.scope.as_deref();
222214
let verifier =

‎src/cmd/delete.rs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,14 @@ impl Delete {
1515
pub async fn run(self) -> anyhow::Result<()> {
1616
log::debug!("deleting client: {}", self.name);
1717

18-
let mut config = Config::load(self.config.as_deref())?;
19-
20-
if config.clients.remove(&self.name).is_some() {
21-
log::info!("deleted client: {}", self.name);
22-
config.store(self.config.as_deref())?;
23-
} else {
24-
log::info!("client did not exist: {}", self.name);
25-
}
26-
27-
Ok(())
18+
Config::locked(self.config.as_deref(), async |config| {
19+
if config.clients.remove(&self.name).is_some() {
20+
log::info!("deleted client: {}", self.name);
21+
} else {
22+
log::info!("client did not exist: {}", self.name);
23+
}
24+
Ok(())
25+
})
26+
.await
2827
}
2928
}

‎src/cmd/mcp.rs‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -82,38 +82,38 @@ impl OidcMcpServer {
8282
&self,
8383
Parameters(params): Parameters<GetTokenParams>,
8484
) -> Result<CallToolResult, rmcp::ErrorData> {
85-
let mut config = Config::load(self.config_path.as_deref()).map_err(|e| {
86-
rmcp::ErrorData::internal_error(format!("failed to load config: {e}"), None)
87-
})?;
88-
89-
let client = config.by_name_mut(&params.name).ok_or_else(|| {
90-
rmcp::ErrorData::invalid_params(format!("unknown client '{}'", params.name), None)
91-
})?;
92-
93-
let token = get_token(client, &self.http).await.map_err(|e| {
94-
rmcp::ErrorData::internal_error(format!("failed to get token: {e}"), None)
95-
})?;
85+
let http = self.http.clone();
86+
let token_type = params.token_type.clone();
87+
88+
let token_value = Config::locked(self.config_path.as_deref(), async |config| {
89+
let client = config
90+
.by_name_mut(&params.name)
91+
.ok_or_else(|| anyhow::anyhow!("unknown client '{}'", params.name))?;
92+
93+
let token = get_token(client, &http).await?;
94+
95+
let state = match token {
96+
TokenResult::Refreshed(state) => {
97+
client.state = Some(state.clone());
98+
state
99+
}
100+
TokenResult::Existing(state) => state,
101+
};
96102

97-
let state = match token {
98-
TokenResult::Refreshed(state) => {
99-
client.state = Some(state.clone());
100-
config.store(self.config_path.as_deref()).map_err(|e| {
101-
rmcp::ErrorData::internal_error(format!("failed to store config: {e}"), None)
102-
})?;
103-
state
104-
}
105-
TokenResult::Existing(state) => state,
106-
};
103+
let token_value = match token_type.as_str() {
104+
"id" => state
105+
.id_token
106+
.ok_or_else(|| anyhow::anyhow!("ID token not available"))?,
107+
"refresh" => state
108+
.refresh_token
109+
.ok_or_else(|| anyhow::anyhow!("refresh token not available"))?,
110+
_ => state.access_token,
111+
};
107112

108-
let token_value = match params.token_type.as_str() {
109-
"id" => state
110-
.id_token
111-
.ok_or_else(|| rmcp::ErrorData::invalid_params("ID token not available", None))?,
112-
"refresh" => state.refresh_token.ok_or_else(|| {
113-
rmcp::ErrorData::invalid_params("refresh token not available", None)
114-
})?,
115-
_ => state.access_token,
116-
};
113+
Ok(token_value)
114+
})
115+
.await
116+
.map_err(|e| rmcp::ErrorData::internal_error(format!("{e}"), None))?;
117117

118118
Ok(CallToolResult::success(vec![ContentBlock::text(
119119
token_value,

0 commit comments

Comments
 (0)