Skip to content

Commit 3d441e7

Browse files
authored
fix(config): reject unknown fields in nested gateway config tables (#1666)
* fix(config): reject unknown fields in nested gateway config tables The gateway TOML loader silently ignored keys placed under the wrong table header. PR #1661 fixed one instance of this (supervisor_image under [openshell.gateway.gateway_jwt]) but the root cause remained: the nested gateway config tables did not deny unknown fields, so a misplaced key was accepted and dropped instead of erroring. Concretely, tasks/scripts/gateway.sh emitted `sandbox_namespace` right after the [openshell.gateway.gateway_jwt] heredoc, so it landed inside the gateway_jwt table rather than [openshell.gateway]. The k8s driver already receives the namespace via [openshell.drivers.kubernetes], so the stray line was dead config that parsed without complaint. Changes: - Add #[serde(deny_unknown_fields)] to the nested gateway config tables that are part of the config-file parse tree: TlsConfig, OidcConfig, MtlsAuthConfig, GatewayAuthConfig, GatewayJwtConfig. - Remove the misplaced sandbox_namespace line from gateway.sh. - Drop the unused Serialize/Deserialize derives from Config and ServiceRoutingConfig (see below). - Add a regression test asserting a key under the wrong nested table is rejected.
1 parent 29e2539 commit 3d441e7

3 files changed

Lines changed: 35 additions & 17 deletions

File tree

crates/openshell-core/src/config.rs

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -172,22 +172,25 @@ fn is_unix_socket(path: &Path) -> bool {
172172
}
173173

174174
/// Server configuration.
175-
#[derive(Debug, Clone, Serialize, Deserialize)]
175+
///
176+
/// Built programmatically in [`crate::Config::new`] and the gateway CLI from
177+
/// the parsed config file, env vars, and CLI flags. It is never deserialized
178+
/// directly; the on-disk config schema lives in the gateway's `config_file`
179+
/// module ([`crate::TlsConfig`] and the other nested tables carry their own
180+
/// `Deserialize` impls for that purpose).
181+
#[derive(Debug, Clone)]
176182
pub struct Config {
177183
/// Address to bind the server to.
178-
#[serde(default = "default_bind_address")]
179184
pub bind_address: SocketAddr,
180185

181186
/// Address to bind the unauthenticated health endpoint to.
182187
///
183188
/// When `None`, the dedicated health listener is disabled.
184-
#[serde(default)]
185189
pub health_bind_address: Option<SocketAddr>,
186190

187191
/// Address to bind the Prometheus metrics endpoint to.
188192
///
189193
/// When `None`, the dedicated metrics listener is disabled.
190-
#[serde(default)]
191194
pub metrics_bind_address: Option<SocketAddr>,
192195

193196
/// Additional bind addresses that serve the same multiplexed gRPC/HTTP
@@ -196,36 +199,30 @@ pub struct Config {
196199
/// Compute drivers may register extra listeners during startup so that
197200
/// sandbox workloads can call back into the gateway over an interface
198201
/// that the operator-supplied `bind_address` does not expose.
199-
#[serde(default)]
200202
pub extra_bind_addresses: Vec<SocketAddr>,
201203

202204
/// Log level (trace, debug, info, warn, error).
203-
#[serde(default = "default_log_level")]
204205
pub log_level: String,
205206

206207
/// TLS configuration. When `None`, the server listens on plaintext HTTP.
207208
pub tls: Option<TlsConfig>,
208209

209210
/// OIDC configuration. When `Some`, the server validates Bearer JWTs.
210-
#[serde(default)]
211211
pub oidc: Option<OidcConfig>,
212212

213213
/// Gateway user authentication behavior.
214-
#[serde(default)]
215214
pub auth: GatewayAuthConfig,
216215

217216
/// mTLS user authentication configuration. When enabled, a verified TLS
218217
/// client certificate can authenticate CLI/SDK callers as a
219218
/// `Principal::User`. This is for local single-user gateways only;
220219
/// sandbox identity is always carried by gateway-minted sandbox JWTs.
221-
#[serde(default)]
222220
pub mtls_auth: MtlsAuthConfig,
223221

224222
/// Gateway-minted sandbox JWT configuration. When `Some`, the gateway
225223
/// loads the signing key from disk and accepts gateway-issued sandbox
226224
/// JWTs as `Principal::Sandbox`. Required for the per-sandbox identity
227225
/// flow (issue #1354).
228-
#[serde(default)]
229226
pub gateway_jwt: Option<GatewayJwtConfig>,
230227

231228
/// Database URL for persistence.
@@ -236,29 +233,26 @@ pub struct Config {
236233
/// The config shape allows multiple drivers so the gateway can evolve
237234
/// toward multi-backend routing. Current releases require exactly one
238235
/// configured driver.
239-
#[serde(default)]
240236
pub compute_drivers: Vec<ComputeDriverKind>,
241237

242238
/// TTL for SSH session tokens, in seconds. 0 disables expiry.
243-
#[serde(default = "default_ssh_session_ttl_secs")]
244239
pub ssh_session_ttl_secs: u64,
245240

246241
/// Browser-facing sandbox service routing configuration.
247-
#[serde(default)]
248242
pub service_routing: ServiceRoutingConfig,
249243
}
250244

251245
/// Browser-facing sandbox service routing configuration.
252-
#[derive(Debug, Clone, Serialize, Deserialize)]
246+
///
247+
/// Part of the programmatically-built [`Config`]; never deserialized directly.
248+
#[derive(Debug, Clone)]
253249
pub struct ServiceRoutingConfig {
254250
/// Base domains accepted for `sandbox--service.<domain>` routes.
255251
/// The first domain is used when the gateway prints endpoint URLs.
256-
#[serde(default = "default_service_routing_domains")]
257252
pub base_domains: Vec<String>,
258253

259254
/// Enable TLS-enabled loopback gateway listeners to also accept plaintext
260255
/// HTTP for sandbox service hostnames.
261-
#[serde(default = "default_enable_loopback_service_http")]
262256
pub enable_loopback_service_http: bool,
263257
}
264258

@@ -274,6 +268,7 @@ pub struct ServiceRoutingConfig {
274268
/// In both modes, authentication is handled at the application layer
275269
/// (e.g. OIDC bearer tokens). mTLS is an additional mechanism.
276270
#[derive(Debug, Clone, Serialize, Deserialize)]
271+
#[serde(deny_unknown_fields)]
277272
pub struct TlsConfig {
278273
/// Path to the TLS certificate file.
279274
pub cert_path: PathBuf,
@@ -304,6 +299,7 @@ pub struct TlsConfig {
304299
/// - Entra ID / Okta: `roles`
305300
/// - Custom: any dot-separated path into the JWT claims
306301
#[derive(Debug, Clone, Serialize, Deserialize)]
302+
#[serde(deny_unknown_fields)]
307303
pub struct OidcConfig {
308304
/// OIDC issuer URL (e.g., `http://localhost:8180/realms/openshell`).
309305
pub issuer: String,
@@ -338,6 +334,7 @@ pub struct OidcConfig {
338334

339335
/// mTLS user authentication for local, single-user gateways.
340336
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
337+
#[serde(deny_unknown_fields)]
341338
pub struct MtlsAuthConfig {
342339
/// When true, the gateway maps a verified TLS client certificate into a
343340
/// user principal. Keep disabled for Kubernetes deployments because
@@ -348,6 +345,7 @@ pub struct MtlsAuthConfig {
348345

349346
/// Gateway user authentication settings.
350347
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
348+
#[serde(deny_unknown_fields)]
351349
pub struct GatewayAuthConfig {
352350
/// When true, unauthenticated user/CLI calls are accepted as a local
353351
/// developer principal. This is an unsafe local-development escape hatch
@@ -368,6 +366,7 @@ const fn default_jwks_ttl_secs() -> u64 {
368366
/// signing key never leaves the gateway process; the public key is loaded
369367
/// by the same gateway so it can validate its own tokens.
370368
#[derive(Debug, Clone, Serialize, Deserialize)]
369+
#[serde(deny_unknown_fields)]
371370
pub struct GatewayJwtConfig {
372371
/// Path to the Ed25519 signing key (PKCS#8 PEM).
373372
pub signing_key_path: PathBuf,

crates/openshell-server/src/config_file.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,26 @@ nonsense = true
420420
assert!(matches!(err, ConfigFileError::Parse { .. }));
421421
}
422422

423+
#[test]
424+
fn rejects_unknown_field_in_nested_gateway_jwt_table() {
425+
// Regression guard for the class of silent-misconfig bug fixed in
426+
// PR #1661: a key indented under the wrong table header (here,
427+
// `sandbox_namespace` landing under `[openshell.gateway.gateway_jwt]`
428+
// instead of `[openshell.gateway]`) must be rejected rather than
429+
// silently ignored.
430+
let toml = r#"
431+
[openshell.gateway.gateway_jwt]
432+
signing_key_path = "/tmp/jwt/signing.pem"
433+
public_key_path = "/tmp/jwt/public.pem"
434+
kid_path = "/tmp/jwt/kid"
435+
sandbox_namespace = "agents"
436+
"#;
437+
let tmp = write_tmp(toml);
438+
let err = load(tmp.path())
439+
.expect_err("unknown field in nested gateway_jwt table must be rejected");
440+
assert!(matches!(err, ConfigFileError::Parse { .. }));
441+
}
442+
423443
#[test]
424444
fn rejects_removed_ssh_endpoint_fields() {
425445
let toml = r"

tasks/scripts/gateway.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,6 @@ EOF
333333
case "${DRIVER}" in
334334
kubernetes)
335335
cat >>"${CONFIG_PATH}" <<EOF
336-
sandbox_namespace = "${SANDBOX_NAMESPACE}"
337336
338337
[openshell.drivers.kubernetes]
339338
namespace = "${SANDBOX_NAMESPACE}"

0 commit comments

Comments
 (0)