8000 feat(minvmd-net): add network policy enforcement hook by gominimal-aw-bot[bot] · Pull Request #471 · gominimal/minimal · GitHub
[go: up one dir, main page]
Skip to content

feat(minvmd-net): add network policy enforcement hook #471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gominimal-aw-bot wants to merge 1 commit into from
Closed

feat(minvmd-net): add network policy enforcement hook #471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions crates/minvmd/src/cmd/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,13 @@ fn run_foreground() -> Result<()> {
.set_nonblocking(false)
.context("setting listener to blocking")?;

// Wire the taskspec `network` declaration here once the capability envelope
// (see capability-envelope tracking issue) is available. Replace
// `NetworkPolicy::Open` with the resolved policy from the task's network
// declaration.
crate::net::check_network_policy(&crate::net::NetworkPolicy::Open)
.context("network policy check failed")?;

let exe = std::env::current_exe().context("resolving current executable path")?;
let mut child = std::process::Command::new(&exe)
.arg("__krun-vmm")
Expand Down
40 changes: 40 additions & 0 deletions crates/minvmd/src/net.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
//! [`gvproxy`] crate. The transport-agnostic gvproxy spawn/lookup helpers come
//! from that crate; `gvproxy_bin` below wraps the lookup with `minvmd`'s own
//! override variable.
//!
//! This module also defines the network policy enforcement hook (R3.1, R3.2, R3.3).
//! `check_network_policy` enforces the VM session's network access policy before boot.
//! Currently it is a no-op (policy default: open / allow all).

use std::path::PathBuf;

Expand Down Expand Up @@ -63,6 +67,31 @@ pub fn gvproxy_bin() -> PathBuf {
gvproxy::gvproxy_bin("MINVMD_GVPROXY_PATH")
}

/// The network access policy for a VM session.
///
/// `Open` permits all outbound connections. `Allowlist` is reserved for
/// future enforcement once the taskspec `network` declaration is wired in.
#[derive(Debug, Clone)]
pub enum NetworkPolicy {
/// Allow all outbound network connections.
Open,
/// Restrict outbound connections to a set of hostnames or CIDR ranges.
/// Unused in v1; reserved for future enforcement.
Allowlist(Vec<String>),
}

/// Check whether the given network policy permits the session to start.
///
/// Currently always returns `Ok(())` (policy default: open).
/// A future spec will implement allowlist enforcement once the taskspec `network`
/// declaration is available.
pub fn check_network_policy(policy: &NetworkPolicy) -> anyhow::Result<()> {
@gominimal-aw-bot gominimal-aw-bot Bot Jun 19, 2026
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 · Spec compliance · R3.1

The spec (R3.1) requires the function to return Result<(), PolicyError> where PolicyError is a new typed error variant—either a variant added to crate::error or a dedicated net::PolicyError newtype. The spec is explicit that it "is unused in v1 but must be defined so future callers can match on it."

The implementation returns anyhow::Result<()> and no PolicyError type exists in error.rs or elsewhere in the crate. This has two consequences:

  1. API break when enforcement lands. Today callers call .context("network policy check failed")? against an anyhow::Result, which compiles fine. When a future PR implements real enforcement and changes the return to Result<(), PolicyError>, every call site must be updated—a wider blast radius than if the typed error were defined now (even as a stub).
  2. Future callers cannot match on policy errors. The spec's rationale for requiring the typed error is precisely to allow matching at the call site (e.g., to distinguish "policy denied outbound" from an I/O failure). anyhow::Error erases that information.

Suggested fix—add a PolicyError newtype in net.rs (or a variant in error.rs) and change the signature:

/// Returned when a network policy check refuses session startup.
///
/// Unused in v1; defined so future callers can match on it.
#[derive(Debug, thiserror::Error)]
#[non_exhaustive]
pub enum PolicyError {
    #[error("network policy denied: {reason}")]
    Denied { reason: String },
}

pub fn check_network_policy(policy: &NetworkPolicy) -> Result<(), PolicyError> {
    match policy {
        NetworkPolicy::Open => Ok(()),
        NetworkPolicy::Allowlist(_) => Ok(()), // no-op in v1
    }
}

The call site in run.rs still compiles with .context("network policy check failed")? because PolicyError: std::error::Error and anyhow converts it via From.

match policy {
NetworkPolicy::Open => Ok(()),
NetworkPolicy::Allowlist(_) => Ok(()), // no-op in v1
}
}

#[cfg(test)]
mod tests {
use std::sync::Mutex;
Expand Down Expand Up @@ -120,4 +149,15 @@ mod tests {
unsafe { std::env::remove_var("MINVMD_GVPROXY_PATH") };
assert_eq!(gvproxy_bin(), PathBuf::from("gvproxy"));
}

#[test]
fn check_network_policy_open_is_ok() {
assert!(check_network_policy(&NetworkPolicy::Open).is_ok());
}

#[test]
fn check_network_policy_allowlist_is_ok() {
let policy = NetworkPolicy::Allowlist(vec!["example.com".to_string()]);
assert!(check_network_policy(&policy).is_ok());
}
}
Loading
0