Skip to content

Commit 488c013

Browse files
authored
Merge pull request #389 from intendednull/audit/agent-sec
fix(agent): const-time bearer + scope gates
2 parents 6e92492 + 13c7dc6 commit 488c013

4 files changed

Lines changed: 55 additions & 4 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/agent/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ tracing-subscriber = { workspace = true }
3232
anyhow = { workspace = true }
3333
rand = "0.8"
3434
dirs = "6"
35+
subtle = "2"
3536

3637
[dev-dependencies]
3738
willow-client = { path = "../client", features = ["test-utils"] }

crates/agent/src/main.rs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
//! notifications to AI agents, bots, and scripts. The agent binary is a
55
//! first-class Willow peer with its own Ed25519 identity.
66
7-
use clap::Parser;
7+
use clap::{Parser, ValueEnum};
8+
use willow_agent::scopes::TokenScope;
89
use willow_agent::{auth, server};
910
use willow_client::{ClientConfig, ClientHandle};
1011
use willow_identity::Identity;
@@ -57,6 +58,10 @@ struct Cli {
5758
#[arg(long, default_value = "info")]
5859
log_level: String,
5960

61+
/// Token scope for HTTP transport (least-privilege default).
62+
#[arg(long, value_enum, default_value_t = ScopeArg::Messaging)]
63+
scope: ScopeArg,
64+
6065
/// Generate a new identity and exit.
6166
#[arg(long)]
6267
generate_identity: bool,
@@ -66,6 +71,27 @@ struct Cli {
6671
print_peer_id: bool,
6772
}
6873

74+
/// CLI-friendly enum for `--scope`. Maps to `TokenScope`.
75+
#[derive(Copy, Clone, Debug, ValueEnum)]
76+
enum ScopeArg {
77+
/// Messaging tools only (least-privilege default).
78+
Messaging,
79+
/// Read-only: no tools, all resources.
80+
Read,
81+
/// Full access: all tools, all resources.
82+
Full,
83+
}
84+
85+
impl From<ScopeArg> for TokenScope {
86+
fn from(s: ScopeArg) -> Self {
87+
match s {
88+
ScopeArg::Messaging => TokenScope::Messaging,
89+
ScopeArg::Read => TokenScope::ReadOnly,
90+
ScopeArg::Full => TokenScope::Full,
91+
}
92+
}
93+
}
94+
6995
#[tokio::main]
7096
async fn main() -> anyhow::Result<()> {
7197
let cli = Cli::parse();
@@ -148,7 +174,7 @@ async fn main() -> anyhow::Result<()> {
148174
}
149175
"http" => {
150176
tracing::info!("starting MCP HTTP server on {}", cli.bind);
151-
server::serve_http(client, &cli.bind, Default::default(), token).await?;
177+
server::serve_http(client, &cli.bind, cli.scope.into(), token).await?;
152178
}
153179
other => {
154180
anyhow::bail!("unsupported transport: {other} (supported: 'stdio', 'http')");

crates/agent/src/server.rs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,16 @@ impl<N: Network> ServerHandler for WillowMcpServer<N> {
163163
request: ReadResourceRequestParams,
164164
_context: RequestContext<RoleServer>,
165165
) -> impl Future<Output = Result<ReadResourceResult, ErrorData>> + Send + '_ {
166-
async move { resources::read_resource(&self.client, &request.uri).await }
166+
async move {
167+
if !self.scope.allows_resource(&request.uri) {
168+
return Err(ErrorData::new(
169+
ErrorCode::INVALID_REQUEST,
170+
format!("resource '{}' not allowed by token scope", request.uri),
171+
None,
172+
));
173+
}
174+
resources::read_resource(&self.client, &request.uri).await
175+
}
167176
}
168177
}
169178

@@ -232,7 +241,7 @@ async fn bearer_auth_middleware(
232241
match auth_header {
233242
Some(value) if value.starts_with("Bearer ") => {
234243
let provided = &value["Bearer ".len()..];
235-
if provided == expected_token {
244+
if tokens_eq_ct(provided.as_bytes(), expected_token.as_bytes()) {
236245
next.run(req).await
237246
} else {
238247
(StatusCode::FORBIDDEN, "invalid bearer token").into_response()
@@ -242,6 +251,20 @@ async fn bearer_auth_middleware(
242251
}
243252
}
244253

254+
/// Constant-time equality check for bearer tokens.
255+
///
256+
/// Length is compared first to avoid leaking the expected length via timing,
257+
/// then `subtle::ConstantTimeEq` performs a branch-free byte-wise comparison.
258+
/// This prevents timing side-channel attacks where an attacker measures
259+
/// response latency to recover the token byte by byte (issues #301, #304).
260+
fn tokens_eq_ct(provided: &[u8], expected: &[u8]) -> bool {
261+
use subtle::ConstantTimeEq;
262+
if provided.len() != expected.len() {
263+
return false;
264+
}
265+
provided.ct_eq(expected).into()
266+
}
267+
245268
use axum::response::IntoResponse;
246269

247270
#[cfg(test)]

0 commit comments

Comments
 (0)