Skip to content

Commit e7fe4af

Browse files
authored
Merge commit from fork
* fix(sandbox): prevent session resource collisions Sandbox IDs previously replaced unsupported session-key characters with dashes, allowing distinct caller-controlled keys to select the same container and persisted home. Derive a versioned SHA-256 identity from the scope and complete raw key so identical sessions remain stable while distinct keys cannot alias through normalization.\n\nPin the identity format and cover the reported channel-key collision across Docker, Apple Container, and session home paths. Document the intentional legacy-home break and shared-home trust boundary. * docs(security): credit sandbox advisory reporter Add the sandbox isolation fix to the Unreleased security notes and thank @DavidCarliez for the responsible disclosure.
1 parent f640c9b commit e7fe4af

4 files changed

Lines changed: 134 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919

2020
### Security
2121

22+
- [sandbox] Prevent session keys from sharing sandbox containers and persisted
23+
homes. Thanks to [@DavidCarliez](https://github.com/DavidCarliez) for
24+
responsibly reporting this issue.
25+
2226
## [20260902.02] - 2026-09-02
2327

2428
### Security

crates/tools/src/sandbox/router.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use std::{
88
use {
99
async_trait::async_trait,
1010
secrecy::ExposeSecret,
11+
sha2::{Digest, Sha256},
1112
tokio::sync::RwLock,
1213
tracing::{debug, info, warn},
1314
};
@@ -907,22 +908,21 @@ impl SandboxRouter {
907908
self.agent_overrides.write().await.remove(session_key);
908909
}
909910

910-
/// Derive a SandboxId for a given session key.
911-
/// The key is sanitized for use as a container name (only alphanumeric, dash, underscore, dot).
911+
/// Derive a collision-resistant SandboxId for a given session key.
912912
pub fn sandbox_id_for(&self, session_key: &str) -> SandboxId {
913-
let sanitized: String = session_key
914-
.chars()
915-
.map(|c| {
916-
if c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.' {
917-
c
918-
} else {
919-
'-'
920-
}
921-
})
922-
.collect();
913+
let scope: &[u8] = match &self.config.scope {
914+
super::types::SandboxScope::Session => b"session",
915+
super::types::SandboxScope::Agent => b"agent",
916+
super::types::SandboxScope::Shared => b"shared",
917+
};
918+
let mut hasher = Sha256::new();
919+
hasher.update(b"moltis-sandbox-id-v2\0");
920+
hasher.update(scope);
921+
hasher.update(b"\0");
922+
hasher.update(session_key.as_bytes());
923923
SandboxId {
924924
scope: self.config.scope.clone(),
925-
key: sanitized,
925+
key: format!("v2-{:x}", hasher.finalize()),
926926
}
927927
}
928928

crates/tools/src/sandbox/tests/docker_router.rs

Lines changed: 103 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -499,10 +499,109 @@ fn test_sandbox_router_sandbox_id_for() {
499499
};
500500
let router = SandboxRouter::new(config);
501501
let id = router.sandbox_id_for("session:abc");
502-
assert_eq!(id.key, "session-abc");
503-
// Plain alphanumeric keys pass through unchanged.
504-
let id2 = router.sandbox_id_for("main");
505-
assert_eq!(id2.key, "main");
502+
let repeated = router.sandbox_id_for("session:abc");
503+
504+
assert_eq!(id.key, repeated.key);
505+
assert_eq!(
506+
id.key,
507+
"v2-20562047619de23f04775e9dc09eb3070a7bb944d80c2e336b71e2b603dd95d7"
508+
);
509+
assert!(id.key.starts_with("v2-"));
510+
assert_eq!(id.key.len(), 67);
511+
assert!(
512+
id.key
513+
.strip_prefix("v2-")
514+
.is_some_and(|digest| digest.chars().all(|ch| ch.is_ascii_hexdigit()))
515+
);
516+
}
517+
518+
#[test]
519+
fn test_sandbox_router_session_keys_cannot_collide() {
520+
let config = SandboxConfig {
521+
scope: SandboxScope::Session,
522+
home_persistence: HomePersistence::Session,
523+
container_prefix: Some("moltis-test-sandbox".into()),
524+
..Default::default()
525+
};
526+
let router = SandboxRouter::new(config.clone());
527+
let victim =
528+
router.sandbox_id_for("agent:default:channel:telegram:account:acct1:peer:user:12345");
529+
let attacker =
530+
router.sandbox_id_for("agent-default-channel-telegram-account-acct1-peer-user-12345");
531+
532+
assert_ne!(victim.key, attacker.key);
533+
534+
let docker = DockerSandbox::new(config.clone());
535+
assert_ne!(
536+
docker.container_name(&victim),
537+
docker.container_name(&attacker)
538+
);
539+
540+
assert_ne!(
541+
apple_container_name("moltis-test-sandbox", &victim.key, 0),
542+
apple_container_name("moltis-test-sandbox", &attacker.key, 0)
543+
);
544+
545+
let victim_home = sandbox_home_persistence_host_dir(&config, None, &victim).unwrap();
546+
let attacker_home = sandbox_home_persistence_host_dir(&config, None, &attacker).unwrap();
547+
assert_ne!(victim_home, attacker_home);
548+
assert_eq!(
549+
victim_home.parent(),
550+
Some(
551+
moltis_config::data_dir()
552+
.join("sandbox/home/session")
553+
.as_path()
554+
)
555+
);
556+
}
557+
558+
#[test]
559+
fn test_sandbox_router_reserved_path_keys_are_safe() {
560+
let router = SandboxRouter::new(SandboxConfig {
561+
scope: SandboxScope::Session,
562+
..Default::default()
563+
});
564+
565+
for session_key in ["", ".", "..", "/", "session:abc", "session-abc"] {
566+
let id = router.sandbox_id_for(session_key);
567+
assert_ne!(id.key, ".");
568+
assert_ne!(id.key, "..");
569+
assert!(!id.key.is_empty());
570+
}
571+
572+
assert_ne!(
573+
router.sandbox_id_for("session:abc").key,
574+
router.sandbox_id_for("session-abc").key
575+
);
576+
}
577+
578+
#[test]
579+
fn test_sandbox_router_scope_is_part_of_identity() {
580+
let session_router = SandboxRouter::new(SandboxConfig {
581+
scope: SandboxScope::Session,
582+
..Default::default()
583+
});
584+
let agent_router = SandboxRouter::new(SandboxConfig {
585+
scope: SandboxScope::Agent,
586+
..Default::default()
587+
});
588+
let shared_router = SandboxRouter::new(SandboxConfig {
589+
scope: SandboxScope::Shared,
590+
..Default::default()
591+
});
592+
593+
assert_eq!(
594+
session_router.sandbox_id_for("session:abc").key,
595+
"v2-20562047619de23f04775e9dc09eb3070a7bb944d80c2e336b71e2b603dd95d7"
596+
);
597+
assert_eq!(
598+
agent_router.sandbox_id_for("session:abc").key,
599+
"v2-e6a91e10ded6e80f84e1bab8c28c117700c9eef4672ac5632469c77c7a2b7829"
600+
);
601+
assert_eq!(
602+
shared_router.sandbox_id_for("session:abc").key,
603+
"v2-655b7035267c8dd21b19326dbcf221ef75d502f246c32fe5c5c522eb427ba2af"
604+
);
506605
}
507606

508607
#[tokio::test]

docs/src/sandbox.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,23 @@ home_persistence = "session" # "off", "session", or "shared" (default)
363363
```
364364

365365
- `off`: no home mount, container home is ephemeral
366-
- `session`: mount a per-session host folder to `/home/sandbox`
366+
- `session`: mount a per-session host folder to `/home/sandbox`; folder names
367+
are opaque, collision-resistant identifiers rather than raw session keys
367368
- `shared`: mount one shared host folder to `/home/sandbox` for all sessions
368369
(defaults to `data_dir()/sandbox/home/shared`, or `shared_home_dir` if set)
369370

370-
Moltis stores persisted homes under `data_dir()/sandbox/home/`.
371+
Moltis stores persisted homes under `data_dir()/sandbox/home/`. Per-session
372+
folder names are opaque, collision-resistant identifiers; do not derive or
373+
depend on them directly. The `shared` mode is intentionally visible to every
374+
sandbox session and is not a session-isolation boundary. Use `session` or `off`
375+
for credentials or other private state when sessions have different trust
376+
levels.
377+
378+
After upgrading from a version that used readable session folder names, gateway
379+
startup discards legacy instance containers. Legacy home folders remain on disk
380+
but are not automatically reused because multiple session keys may have mapped
381+
to the same folder. Administrators can inspect and migrate trusted content
382+
manually.
371383

372384
## Managed Files mount
373385

0 commit comments

Comments
 (0)