Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions architecture/gateway.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ health, metrics, or tunnel routes. The plaintext service router also rejects
browser requests whose Fetch Metadata, Origin, or Referer headers indicate a
cross-origin or sibling-subdomain request.

Operators can configure a gateway-wide gRPC request rate limit. The limit is
applied only to gRPC API traffic after protocol multiplexing; health, metrics,
and local sandbox-service HTTP routes are not rate limited by this control.

Supported auth modes:

| Mode | Use |
Expand Down
64 changes: 63 additions & 1 deletion crates/openshell-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use std::os::unix::fs::FileTypeExt;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::str::FromStr;
#[cfg(unix)]
use std::time::Duration;

// ── Public default constants ────────────────────────────────────────────
Expand Down Expand Up @@ -364,6 +363,20 @@ pub struct Config {
/// TTL for SSH session tokens, in seconds. 0 disables expiry.
pub ssh_session_ttl_secs: u64,

/// Maximum gRPC requests allowed per rate-limit window.
///
/// When paired with [`Self::grpc_rate_limit_window_secs`], positive values
/// enable gateway-wide gRPC request rate limiting. `None` or `0` disables
/// the limit.
pub grpc_rate_limit_requests: Option<u64>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question: Why not a nested type?


/// gRPC rate-limit window length in seconds.
///
/// When paired with [`Self::grpc_rate_limit_requests`], positive values
/// enable gateway-wide gRPC request rate limiting. `None` or `0` disables
/// the limit.
pub grpc_rate_limit_window_secs: Option<u64>,

/// Browser-facing sandbox service routing configuration.
pub service_routing: ServiceRoutingConfig,
}
Expand Down Expand Up @@ -547,6 +560,8 @@ impl Config {
database_url: String::new(),
compute_drivers: vec![],
ssh_session_ttl_secs: default_ssh_session_ttl_secs(),
grpc_rate_limit_requests: None,
grpc_rate_limit_window_secs: None,
service_routing: ServiceRoutingConfig::default(),
}
}
Expand Down Expand Up @@ -614,6 +629,29 @@ impl Config {
self
}

/// Set the gateway-wide gRPC request rate limit.
#[must_use]
pub const fn with_grpc_rate_limit(
mut self,
requests: Option<u64>,
window_secs: Option<u64>,
) -> Self {
self.grpc_rate_limit_requests = requests;
self.grpc_rate_limit_window_secs = window_secs;
self
}

/// Return the effective gRPC rate limit, if fully configured and enabled.
#[must_use]
pub fn grpc_rate_limit(&self) -> Option<(u64, Duration)> {
let requests = self.grpc_rate_limit_requests?;
let window_secs = self.grpc_rate_limit_window_secs?;
if requests == 0 || window_secs == 0 {
None
} else {
Some((requests, Duration::from_secs(window_secs)))
}
}
/// Set the OIDC configuration for JWT-based authentication.
#[must_use]
pub fn with_oidc(mut self, oidc: OidcConfig) -> Self {
Expand Down Expand Up @@ -737,6 +775,7 @@ mod tests {
#[cfg(unix)]
use std::os::unix::net::UnixListener;
use std::path::PathBuf;
use std::time::Duration;

#[test]
fn compute_driver_kind_parses_supported_values() {
Expand Down Expand Up @@ -794,6 +833,29 @@ mod tests {
assert_eq!(cfg.ttl_secs, 0);
}

#[test]
fn grpc_rate_limit_requires_positive_pair() {
assert!(Config::new(None).grpc_rate_limit().is_none());
assert!(
Config::new(None)
.with_grpc_rate_limit(Some(10), None)
.grpc_rate_limit()
.is_none()
);
assert!(
Config::new(None)
.with_grpc_rate_limit(Some(0), Some(60))
.grpc_rate_limit()
.is_none()
);
assert_eq!(
Config::new(None)
.with_grpc_rate_limit(Some(10), Some(60))
.grpc_rate_limit(),
Some((10, Duration::from_secs(60)))
);
}

#[test]
fn service_routing_allows_loopback_plaintext_http_by_default() {
let cfg = Config::new(None);
Expand Down
121 changes: 121 additions & 0 deletions crates/openshell-server/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,14 @@ struct RunArgs {
#[arg(long, env = "OPENSHELL_OIDC_SCOPES_CLAIM", default_value = "")]
oidc_scopes_claim: String,

/// Maximum gRPC requests allowed per rate-limit window. Set to 0 to disable.
#[arg(long, env = "OPENSHELL_GRPC_RATE_LIMIT_REQUESTS")]
grpc_rate_limit_requests: Option<u64>,

/// gRPC rate-limit window length in seconds. Set to 0 to disable.
#[arg(long, env = "OPENSHELL_GRPC_RATE_LIMIT_WINDOW_SECONDS")]
grpc_rate_limit_window_seconds: Option<u64>,

/// Subject Alternative Names configured on the gateway server certificate.
/// Wildcard DNS SANs also enable sandbox service URLs under that domain.
#[arg(
Expand Down Expand Up @@ -353,8 +361,16 @@ async fn run_from_args(mut args: RunArgs, matches: ArgMatches) -> Result<()> {
config = config
.with_database_url(db_url)
.with_compute_drivers(args.drivers.clone())
.with_grpc_rate_limit(
args.grpc_rate_limit_requests,
args.grpc_rate_limit_window_seconds,
)
.with_server_sans(args.server_sans.clone())
.with_loopback_service_http(args.enable_loopback_service_http);
validate_grpc_rate_limit_args(
args.grpc_rate_limit_requests,
args.grpc_rate_limit_window_seconds,
)?;

if let Some(ttl) = file
.as_ref()
Expand Down Expand Up @@ -608,6 +624,37 @@ fn merge_file_into_args(args: &mut RunArgs, file: &GatewayFileSection, matches:
args.oidc_scopes_claim.clone_from(&oidc.scopes_claim);
}
}
if let Some(requests) = file.grpc_rate_limit_requests
&& args.grpc_rate_limit_requests.is_none()
&& arg_defaulted(matches, "grpc_rate_limit_requests")
{
args.grpc_rate_limit_requests = Some(requests);
}
if let Some(window) = file.grpc_rate_limit_window_seconds
&& args.grpc_rate_limit_window_seconds.is_none()
&& arg_defaulted(matches, "grpc_rate_limit_window_seconds")
{
args.grpc_rate_limit_window_seconds = Some(window);
}
}

fn validate_grpc_rate_limit_args(requests: Option<u64>, window_seconds: Option<u64>) -> Result<()> {
let disabled = matches!(requests, Some(0)) || matches!(window_seconds, Some(0));
if disabled {
return Ok(());
}
if matches!(
(requests, window_seconds),
(Some(requests), None) if requests > 0
) || matches!(
(requests, window_seconds),
(None, Some(window_seconds)) if window_seconds > 0
) {
return Err(miette::miette!(
"gRPC rate limiting requires both --grpc-rate-limit-requests and --grpc-rate-limit-window-seconds (TOML keys grpc_rate_limit_requests and grpc_rate_limit_window_seconds) to be positive; set either value to 0 to disable"
));
}
Ok(())
}

fn effective_single_driver(args: &RunArgs) -> Option<ComputeDriverKind> {
Expand Down Expand Up @@ -893,6 +940,41 @@ mod tests {
assert!(cli.run.enable_mtls_auth);
}

#[test]
fn command_parses_grpc_rate_limit_flags() {
let _lock = ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let _g1 = EnvVarGuard::remove("OPENSHELL_GRPC_RATE_LIMIT_REQUESTS");
let _g2 = EnvVarGuard::remove("OPENSHELL_GRPC_RATE_LIMIT_WINDOW_SECONDS");

let cli = Cli::try_parse_from([
"openshell-gateway",
"--db-url",
"sqlite::memory:",
"--grpc-rate-limit-requests",
"120",
"--grpc-rate-limit-window-seconds",
"60",
])
.unwrap();

assert_eq!(cli.run.grpc_rate_limit_requests, Some(120));
assert_eq!(cli.run.grpc_rate_limit_window_seconds, Some(60));
}

#[test]
fn validate_grpc_rate_limit_args_requires_positive_pair() {
assert!(super::validate_grpc_rate_limit_args(None, None).is_ok());
assert!(super::validate_grpc_rate_limit_args(Some(0), None).is_ok());
assert!(super::validate_grpc_rate_limit_args(None, Some(0)).is_ok());
assert!(super::validate_grpc_rate_limit_args(Some(0), Some(60)).is_ok());
assert!(super::validate_grpc_rate_limit_args(Some(120), Some(0)).is_ok());
assert!(super::validate_grpc_rate_limit_args(Some(120), Some(60)).is_ok());
assert!(super::validate_grpc_rate_limit_args(Some(120), None).is_err());
assert!(super::validate_grpc_rate_limit_args(None, Some(60)).is_err());
}

#[test]
fn command_rejects_removed_driver_flags() {
let err = command()
Expand Down Expand Up @@ -1316,6 +1398,45 @@ audience = "openshell-cli"
assert_eq!(args.oidc_audience, "openshell-cli");
}

#[test]
fn file_grpc_rate_limit_populates_args_when_cli_omits() {
let (mut args, matches) =
parse_with_args(&["openshell-gateway", "--db-url", "sqlite::memory:"]);
let file = config_file_from_toml(
r"
[openshell.gateway]
grpc_rate_limit_requests = 100
grpc_rate_limit_window_seconds = 30
",
);
merge_file_into_args(&mut args, &file.openshell.gateway, &matches);

assert_eq!(args.grpc_rate_limit_requests, Some(100));
assert_eq!(args.grpc_rate_limit_window_seconds, Some(30));
}

#[test]
fn cli_grpc_rate_limit_overrides_file_value() {
let (mut args, matches) = parse_with_args(&[
"openshell-gateway",
"--db-url",
"sqlite::memory:",
"--grpc-rate-limit-requests",
"20",
]);
let file = config_file_from_toml(
r"
[openshell.gateway]
grpc_rate_limit_requests = 100
grpc_rate_limit_window_seconds = 30
",
);
merge_file_into_args(&mut args, &file.openshell.gateway, &matches);

assert_eq!(args.grpc_rate_limit_requests, Some(20));
assert_eq!(args.grpc_rate_limit_window_seconds, Some(30));
}

#[test]
fn aux_listener_preserves_file_ip_against_public_bind() {
use std::net::SocketAddr;
Expand Down
8 changes: 8 additions & 0 deletions crates/openshell-server/src/config_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ pub struct GatewayFileSection {
pub sandbox_namespace: Option<String>,
#[serde(default)]
pub ssh_session_ttl_secs: Option<u64>,
#[serde(default)]
pub grpc_rate_limit_requests: Option<u64>,
#[serde(default)]
pub grpc_rate_limit_window_seconds: Option<u64>,

// ── Service routing ──────────────────────────────────────────────────
/// Subject Alternative Names configured on the gateway server certificate.
Expand Down Expand Up @@ -349,6 +353,8 @@ health_bind_address = "0.0.0.0:8081"
log_level = "info"
compute_drivers = ["kubernetes"]
sandbox_namespace = "agents"
grpc_rate_limit_requests = 120
grpc_rate_limit_window_seconds = 60
Comment on lines +356 to +357

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

Suggested change
grpc_rate_limit_requests = 120
grpc_rate_limit_window_seconds = 60
[openshell.gateway.grpc_rate_limit]
requests = 120
window_seconds = 60

(or are single config options simpler to handle in the agentic sense?)

default_image = "ghcr.io/nvidia/openshell/sandbox:latest"
supervisor_image = "ghcr.io/nvidia/openshell/supervisor:latest"
client_tls_secret_name = "openshell-sandbox-tls"
Expand All @@ -375,6 +381,8 @@ grpc_endpoint = "https://openshell-gateway.agents.svc:8080"
gw.default_image.as_deref(),
Some("ghcr.io/nvidia/openshell/sandbox:latest")
);
assert_eq!(gw.grpc_rate_limit_requests, Some(120));
assert_eq!(gw.grpc_rate_limit_window_seconds, Some(60));
assert!(gw.tls.is_some());
assert!(gw.oidc.is_some());
assert!(file.openshell.drivers.contains_key("kubernetes"));
Expand Down
5 changes: 5 additions & 0 deletions crates/openshell-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ pub struct ServerState {
/// `IssueSandboxToken` bootstrap path. Only present when the gateway
/// runs in-cluster.
pub k8s_sa_authenticator: Option<Arc<auth::k8s_sa::K8sServiceAccountAuthenticator>>,

/// Gateway-wide gRPC request rate limiter shared by every multiplex path.
pub(crate) grpc_rate_limiter: Option<multiplex::GrpcRateLimiter>,
}

fn is_benign_tls_handshake_failure(error: &std::io::Error) -> bool {
Expand Down Expand Up @@ -164,6 +167,7 @@ impl ServerState {
supervisor_sessions: Arc<supervisor_session::SupervisorSessionRegistry>,
oidc_cache: Option<Arc<auth::oidc::JwksCache>>,
) -> Self {
let grpc_rate_limiter = multiplex::GrpcRateLimiter::from_config(&config);
Self {
config,
store,
Expand All @@ -180,6 +184,7 @@ impl ServerState {
sandbox_jwt_issuer: None,
sandbox_jwt_authenticator: None,
k8s_sa_authenticator: None,
grpc_rate_limiter,
}
}
}
Expand Down
Loading
Loading