8000 feat(minimald-rpc,minimald,minimal2): policy types, GetSessionPolicy RPC, and session policy CLI by gominimal-aw-bot[bot] · Pull Request #521 · gominimal/minimal · GitHub
[go: up one dir, main page]

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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 58 additions & 0 deletions crates/minimal2/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ enum Command {
Attach(AttachArgs),
/// Destroy (terminate) a session
Destroy(DestroyArgs),
/// Session inspection subcommands
Session(SessionArgs),
/// Proxy stdio to a daemon UDS socket (used as an SSH ProxyCommand).
#[command(hide = true)]
Proxy(ProxyArgs),
Expand All @@ -41,6 +43,24 @@ enum Command {
Completions(CompletionsArgs),
}

#[derive(Debug, Args)]
struct SessionArgs {
#[command(subcommand)]
command: SessionCommand,
}

#[derive(Debug, Subcommand)]
enum SessionCommand {
/// Print the effective networking policy for a session as JSON
Policy(PolicyArgs),
}

#[derive(Debug, Args)]
struct PolicyArgs {
/// Session identifier (UUID or session name)
session: String,
}

/// Shared arguments all subcommands
#[derive(Debug, Args)]
pub struct GlobalArgs {
Expand Down Expand Up @@ -118,6 +138,9 @@ async fn main() -> Result<(), ()> {
Command::Activate(args) => cmd_activate(&cli.global_args, args).await,
Command::Attach(args) => cmd_attach(&cli.global_args, args).await,
Command::Destroy(args) => cmd_destroy(&cli.global_args, args).await,
Command::Session(SessionArgs {
command: SessionCommand::Policy(args),
}) => cmd_session_policy(&cli.global_args, args).await,
Command::Proxy(args) => cmd_proxy(args).await,
Command::Completions(CompletionsArgs { shell }) => {
let mut cmd = Cli::command();
Expand Down Expand Up @@ -369,6 +392,41 @@ async fn cmd_attach(global: &GlobalArgs, args: AttachArgs) -> Result<(), ()> {
Err(())
}

/// Print the effective networking policy for a session as JSON.
async fn cmd_session_policy(global: &GlobalArgs, args: PolicyArgs) -> Result<(), ()> {
if let Err(e) = autospawn::ensure_minvmd_running() {
eprintln!("Failed to ensure minvmd is running: {e}");
return Err(());
}

let mut client = connect_daemon(global).await?;

use minimald_rpc::{GetSessionPolicy, GetSessionPolicyRequest};
let lookup = if let Ok(id) = sessions::SessionId::parse_str(&args.session) {
GetSessionPolicyRequest::Id(id)
} else {
GetSessionPolicyRequest::Name(args.session.clone())
};

let resp = client
.oneshot_rpc::<GetSessionPolicy>(lookup)
.await
.map_err(|e| eprintln!("GetSessionPolicy RPC failed: {e}"))?;

match resp {
minimald_rpc::Errorable::Ok(policy) => {
let json = serde_json::to_string(&policy)
.map_err(|e| eprintln!("Failed to serialize policy: {e}"))?;
println!("{json}");
Ok(())
}
minimald_rpc::Errorable::Err { error } => {
eprintln!("{error}");
Err(())
}
}
}

/// Destroy (terminate) a session.
async fn cmd_destroy(global: &GlobalArgs, args: DestroyArgs) -> Result<(), ()> {
if let Err(e) = autospawn::ensure_minvmd_running() {
Expand Down
3 changes: 3 additions & 0 deletions crates/minimald-rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ chrono.workspace = true
serde.workspace = true

sessions.workspace = true

[dev-dependencies]
serde_json.workspace = true
160 changes: 160 additions & 0 deletions crates/minimald-rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,3 +276,163 @@ impl OneshotSshRpc for SessionAbort {
type Request<'a> = sessions::wire::request::Abort;
type Response = Errorable<()>;
}

// ---------------------------------------------------------------------------
// Networking policy types (Unit 2: egress, ingress, dynamic port mapping).
// ---------------------------------------------------------------------------

/// A single static ingress port mapping for an `OwnIp` PTask.
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct PortMapping {
/// Host-side port that forwards inbound connections into the PTask.
pub external_port: u16,
/// PTask-side port that receives forwarded connections.
pub internal_port: u16,
/// Transport protocol for this mapping.
pub proto: IpProto,
}

/// Effective egress policy for an `OwnIp` PTask.
///
/// Each field is `None` to mean allow-all for that dimension. Absent `egress`
/// config on a session is equivalent to all-`None` (allow-all).
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
pub struct EgressPolicy {
/// Allowed destination CIDR prefixes; `None` means allow-all subnets.
pub allow_subnets: Option<Vec<String>>,
/// Allowed destination DNS hostnames; `None` means allow-all hosts.
pub allow_dns_hosts: Option<Vec<String>>,
/// Allowed IP protocols; `None` means allow all protocols.
pub allow_protocols: Option<Vec<IpProto>>,
}

/// Effective ingress policy for an `OwnIp` PTask.
///
/// Default (empty `port_mappings`, no `dynamic_allowed_range`) is deny-all-external.
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
pub struct IngressPolicy {
/// Static port mappings applied at PTask launch.
pub port_mappings: Vec<PortMapping>,
/// Inclusive port range within which dynamic port-mapping requests are
/// accepted; `None` means dynamic mapping is disallowed.
pub dynamic_allowed_range: Option<(u16, u16)>,
}

/// The effective networking policy for a named session, as returned by
/// [`GetSessionPolicy`].
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

MEDIUM — correctness

SessionPolicy is missing #[non_exhaustive] while all three sibling types added in this same PR — PortMapping (line 285), EgressPolicy (line 300), and IngressPolicy (line 314) — ca 10BC8 rry it. The workspace coding standard (docs/rust-coding-standards.md) requires #[non_exhaustive] on public enums/structs that may grow.

SessionPolicy is a public RPC response type that is almost certain to grow as the networking stack matures (e.g. a network_mode field, DNS policy, or firewall zone could be added in Units 3–4). Without #[non_exhaustive], adding any field is a semver-breaking change to minimald-rpc.

Suggested fix: add #[non_exhaustive] on the line immediately above this #[derive]:

#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct SessionPolicy {

pub struct SessionPolicy {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LOW · correctness

SessionPolicy is a public struct that will gain fields as the networking stack is built out (effective mode, assigned IP, etc.), but it is missing #[non_exhaustive]. The Rust coding standards require #[non_exhaustive] on public structs that may grow; the three companion types in this file (PortMapping, EgressPolicy, IngressPolicy) all carry it already.

#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct SessionPolicy {

/// Egress policy; `None` when the session is not `OwnIp` or no explicit
/// egress config is present (allow-all).
pub egress: Option<EgressPolicy>,
/// Ingress policy; `None` when the session is not `OwnIp`.
pub ingress: Option<IngressPolicy>,
}

impl SessionPolicy {
pub fn new(egress: Option<EgressPolicy>, ingress: Option<IngressPolicy>) -> Self {
Self { egress, ingress }
}
}

/// An RPC to read the effective networking policy for a session (R2.6).
pub struct GetSessionPolicy;

/// Request for the [`GetSessionPolicy`] RPC.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum GetSessionPolicyRequest {
Name(String),
Id(SessionId),
}

impl OneshotSshRpc for GetSessionPolicy {
const NAME: &'static str = constcat::concat!(RPC_SUBSYSTEM_PREFIX, "GetSessionPolicy");
type Request<'a> = GetSessionPolicyRequest;
type Response = Errorable<SessionPolicy>;
}

/// An RPC for a process inside a PTask to request a dynamic ingress port
/// mapping at runtime (R2.4).
pub struct DynamicPortMap;

/// Request for the [`DynamicPortMap`] RPC.
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize)]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LOW — correctness

DynamicPortMapRequest is missing #[non_exhaustive]. Per R2.4, the daemon must validate the requested port against dynamic_allowed_ports and reject out-of-range requests with a typed error — the struct is likely to gain additional context fields (e.g. a ttl or protocol_hint) as the full R2.4 implementation lands.

For consistency with the other new structs in this PR that correctly carry #[non_exhaustive], add the attribute here too:

#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct DynamicPortMapRequest {

pub struct DynamicPortMapRequest {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LOW · correctness

DynamicPortMapRequest is a public struct missing #[non_exhaustive]. The Rust coding standards require #[non_exhaustive] on public structs that may grow — R2.4's description of the dynamic port-mapping API is still evolving (Open Questions item 3 in the spec), so additional fields are plausible.

#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct DynamicPortMapRequest {

pub id: SessionId,
pub external_port: u16,
pub internal_port: u16,
pub proto: IpProto,
}

impl DynamicPortMapRequest {
pub fn new(id: SessionId, external_port: u16, internal_port: u16, proto: IpProto) -> Self {
Self {
id,
external_port,
internal_port,
proto,
}
}
}

/// Response for the [`DynamicPortMap`] RPC.
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LOW · correctness

DynamicPortMapResponse is a public struct (currently a unit struct) missing #[non_exhaustive]. The Rust coding standards require #[non_exhaustive] on public structs that may grow. Even though it is currently empty, it is likely to gain fields (e.g., the assigned external port) when the handler is wired to live state in U2-T2.

#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct DynamicPortMapResponse;

pub struct DynamicPortMapResponse;

impl OneshotSshRpc for DynamicPortMap {
const NAME: &'static str = constcat::concat!(RPC_SUBSYSTEM_PREFIX, "DynamicPortMap");
type Request<'a> = DynamicPortMapRequest;
type Response = Errorable<DynamicPortMapResponse>;
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn policy_types_are_present_and_serializable() {
// PortMapping construction and round-trip
let mapping = PortMapping {
external_port: 8080,
internal_port: 80,
proto: IpProto::Tcp,
};
let json = serde_json::to_string(&mapping).unwrap();
let rt: PortMapping = serde_json::from_str(&json).unwrap();
assert_eq!(rt, mapping);

// IngressPolicy default and round-trip
let ingress = IngressPolicy {
port_mappings: vec![mapping],
dynamic_allowed_range: Some((10000, 20000)),
};
let json = serde_json::to_string(&ingress).unwrap();
let rt: IngressPolicy = serde_json::from_str(&json).unwrap();
assert_eq!(rt, ingress);

// EgressPolicy default serializes without error
let egress = EgressPolicy::default();
let json = serde_json::to_string(&egress).unwrap();
let _: EgressPolicy = serde_json::from_str(&json).unwrap();

// SessionPolicy with null egress and default ingress matches expected CLI output
let policy = SessionPolicy {
egress: None,
ingress: Some(IngressPolicy::default()),
};
let json = serde_json::to_string(&policy).unwrap();
assert!(json.contains("\"egress\":null"), "got: {json}");
assert!(json.contains("\"port_mappings\":[]"), "got: {json}");
assert!(
json.contains("\"dynamic_allowed_range\":null"),
"got: {json}"
);
}
}
37 changes: 34 additions & 3 deletions crates/minimald/src/rpc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use minimald_rpc::{
CreateSession, CreateSessionResponse, DestroySession, DestroySessionResponse, Errorable,
GetSessionRecord, GetSessionRecordRequest, GetSessionRecordResponse, GetVersion,
GetVersionResponse, ListSessions, ListSessionsEntry, ListSessionsResponse, OneshotSshRpc,
RPC_SUBSYSTEM_PREFIX, RenameSession, RenameSessionResponse,
GetSessionPolicy, GetSessionPolicyRequest, GetSessionRecord, GetSessionRecordRequest,
GetSessionRecordResponse, GetVersion, GetVersionResponse, IngressPolicy, ListSessions,
ListSessionsEntry, ListSessionsResponse, OneshotSshRpc, RPC_SUBSYSTEM_PREFIX, RenameSession,
RenameSessionResponse, SessionPolicy,
};
use russh::{
Channel as RuChannel, ChannelId,
Expand Down Expand Up @@ -190,6 +191,34 @@ async fn serve_destroy_session(s: ServerStateHandle, c: RuChannel<Msg>) {
}
}

async fn serve_get_session_policy(s: ServerStateHandle, c: RuChannel<Msg>) {
let res = GetSessionPolicy
.handle_channel(c, async |req| {
let mngr = s.sessions_manager().await;
let predicate = match req {
GetSessionPolicyRequest::Id(id) => SessionKeyPredicate::Id(id),
GetSessionPolicyRequest::Name(name) => SessionKeyPredicate::Name(name),
};
let record = mngr
.get_record(predicate)
.await
.map_err(|e| ConnectionError::Internal(e.to_string()))?;
match record {
None => Ok(Errorable::Err {
error: "no session found".to_string(),
}),
Some(_) => Ok(Errorable::Ok(SessionPolicy::new(
None,
Some(IngressPolicy::default()),
))),
}
})
.await;
if let Err(e) = res {
tracing::warn!("RPC handler for {} failed: {}", GetSessionPolicy::NAME, e);
}
}

pub(crate) const STREAM_WORKSPACE_FILES: &str =
constcat::concat!(RPC_SUBSYSTEM_PREFIX, "WorkspaceFilesTarZst");

Expand Down Expand Up @@ -264,6 +293,7 @@ pub async fn handle_ssh_rpc(
| CreateSession::NAME
| RenameSession::NAME
| DestroySession::NAME
| GetSessionPolicy::NAME
| STREAM_WORKSPACE_FILES => {
let mut conn_lock = c.lock().await;
let c_hnd = match conn_lock.take(id) {
Expand Down Expand Up @@ -295,6 +325,7 @@ pub async fn handle_ssh_rpc(
CreateSession::NAME => spawn(serve_create_session(s, channel)),
RenameSession::NAME => spawn(serve_rename_session(s, channel)),
DestroySession::NAME => spawn(serve_destroy_session(s, channel)),
GetSessionPolicy::NAME => spawn(serve_get_session_policy(s, channel)),
STREAM_WORKSPACE_FILES => spawn(serve_stream_workspace_files(s, config, channel)),
_ => unreachable!(),
};
Expand Down
Loading
0