trending/OpenShell
1
0
Fork 0
mirror of https://github.com/NVIDIA/OpenShell.git synced 2026-11-04 08:25:55 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(ocsf): address follow-ups from the #4288 review (#5020)

Browse Source 
* fix(ocsf): keep the fallback event context feature-independent

With test-support, the fallback EventContext carried a fake sandbox
identity, so a binary built with that feature unified in, such as by
cargo build --workspace --all-targets, attributed unbound events to a
sandbox that does not exist. Producer tests no longer need it: the
validator accepts top-level container on its own, and the builder
conformance test uses an explicit sandbox context.

Signed-off-by: Kris Hicks <khicks@nvidia.com>

* test(ocsf): pin builder compile_fail doctests to the missing field

The HTTP, Network, Process, and SSH type-state doctests built their
EventContext from a struct literal, so a new EventContext field would
also make them fail to compile and they would keep passing with the
guard removed. Build the context with ctx::ctx() so the missing
required field is the only compile error.

Signed-off-by: Kris Hicks <khicks@nvidia.com>

* refactor(ocsf): share setter macros with type-state builders

The HTTP, Network, Process, and SSH builders carry a type-state
parameter, so they hand-copied the activity, action, disposition,
severity, status, and message setters that the shared macros generate
for other builders. Let the macros take an optional state parameter and
use them on all four, and route both SSH endpoint setters through one
state transition.

Signed-off-by: Kris Hicks <khicks@nvidia.com>

* docs(ocsf): add type_uid to the Network Activity example

OCSF 1.8.0 requires type_uid, so the example that follows the
conformance claim failed validation as written.

Signed-off-by: Kris Hicks <khicks@nvidia.com>

* test(e2e): report the last exec error in the OCSF conformance test

The test discarded the result of the exec that generates traffic, so
when the exec failed, for example without python3 in the image, the
test retried for a minute and then blamed the JSONL copy. Keep the
latest exec error and include it in the missing-classes failure.

Signed-off-by: Kris Hicks <khicks@nvidia.com>

* test(e2e): read the sandbox id label from podman ps JSON

Signed-off-by: Kris Hicks <khicks@nvidia.com>

---------

Signed-off-by: Kris Hicks <khicks@nvidia.com>
This commit is contained in:
krishicks
2026-10-29 00:37:37 +00:00
committed by GitHub
parent 677875fd03
commit f88b58192b
8 changed files with 75 additions and 249 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/openshell-ocsf/src/builders/http.rs
+6 -48
View File
@@ -42,19 +42,9 @@ impl<'a> HttpActivityBuilder<'a, MissingHttpContext> {
/// be built.
///
/// ```compile_fail
/// use openshell_ocsf::{EventContext, EventOrigin, HttpActivityBuilder};
/// use openshell_ocsf::HttpActivityBuilder;
///
/// let ctx = EventContext {
/// sandbox_id: String::new(),
/// sandbox_name: String::new(),
/// container_image: String::new(),
/// hostname: String::new(),
/// product_version: String::new(),
/// proxy_ip: "127.0.0.1".parse().unwrap(),
/// proxy_port: 3128,
/// origin: EventOrigin::Supervisor,
/// };
/// HttpActivityBuilder::new(&ctx).build();
/// HttpActivityBuilder::new(openshell_ocsf::ctx::ctx()).build();
/// ```
#[must_use]
pub fn new(ctx: impl Into<ContextRef<'a>>) -> Self {
@@ -164,24 +154,6 @@ impl<'a, Context> HttpActivityBuilder<'a, Context> {
self
}
#[must_use]
pub fn activity(mut self, id: ActivityId) -> Self {
self.activity = id;
self
}
#[must_use]
pub fn action(mut self, id: ActionId) -> Self {
self.action = Some(id);
self
}
#[must_use]
pub fn disposition(mut self, id: DispositionId) -> Self {
self.disposition = Some(id);
self
}
#[must_use]
pub fn actor_process(mut self, process: crate::objects::Process) -> Self {
self.actor = Some(Actor::process(process));
@@ -199,26 +171,12 @@ impl<'a, Context> HttpActivityBuilder<'a, Context> {
self.firewall_rule = Some(FirewallRule::new(name, rule_type));
self
}
#[must_use]
pub fn severity(mut self, id: SeverityId) -> Self {
self.severity = id;
self
}
#[must_use]
pub fn status(mut self, id: StatusId) -> Self {
self.status = Some(id);
self
}
#[must_use]
pub fn message(mut self, msg: impl Into<String>) -> Self {
self.message = Some(msg.into());
self
}
}
impl_activity_setter!(HttpActivityBuilder<Context>);
impl_action_disposition_setters!(HttpActivityBuilder<Context>);
impl_builder_setters!(HttpActivityBuilder<Context>);
impl HttpActivityBuilder<'_, HasHttpContext> {
#[must_use]
pub fn build(self) -> OcsfEvent {
crates/openshell-ocsf/src/builders/mod.rs
+11 -10
View File
@@ -13,10 +13,11 @@
/// ```ignore
/// impl_builder_setters!(MyBuilder); // severity + status + message
/// impl_builder_setters!(MyBuilder, no_status); // severity + message only
/// impl_builder_setters!(MyBuilder<State>); // for a type-state builder
/// ```
macro_rules! impl_builder_setters {
($builder:ident) => {
impl<'a> $builder<'a> {
($builder:ident $(<$state:ident>)?) => {
impl<'a $(, $state)?> $builder<'a $(, $state)?> {
/// Set the event severity.
#[must_use]
pub fn severity(mut self, id: $crate::enums::SeverityId) -> Self {
@@ -37,8 +38,8 @@ macro_rules! impl_builder_setters {
}
}
};
($builder:ident, no_status) => {
impl<'a> $builder<'a> {
($builder:ident $(<$state:ident>)?, no_status) => {
impl<'a $(, $state)?> $builder<'a $(, $state)?> {
/// Set the event severity.
#[must_use]
pub fn severity(mut self, id: $crate::enums::SeverityId) -> Self {
@@ -58,8 +59,8 @@ macro_rules! impl_builder_setters {
/// Generate the `activity` setter used by every event builder that carries an
/// `activity` field.
macro_rules! impl_activity_setter {
($builder:ident) => {
impl<'a> $builder<'a> {
($builder:ident $(<$state:ident>)?) => {
impl<'a $(, $state)?> $builder<'a $(, $state)?> {
/// Set the event activity identifier.
#[must_use]
pub fn activity(mut self, id: $crate::enums::ActivityId) -> Self {
@@ -70,11 +71,11 @@ macro_rules! impl_activity_setter {
};
}
/// Generate `action` and `disposition` setters shared by network, HTTP, SSH,
/// process, and detection-finding builders.
/// Generate `action` and `disposition` setters shared by every builder whose
/// event records an action and its disposition.
macro_rules! impl_action_disposition_setters {
($builder:ident) => {
impl<'a> $builder<'a> {
($builder:ident $(<$state:ident>)?) => {
impl<'a $(, $state)?> $builder<'a $(, $state)?> {
/// Set the action taken.
#[must_use]
pub fn action(mut self, id: $crate::enums::ActionId) -> Self {
crates/openshell-ocsf/src/builders/network.rs
+6 -48
View File
@@ -44,19 +44,9 @@ impl<'a> NetworkActivityBuilder<'a, MissingNetworkEndpoint> {
/// it can be built.
///
/// ```compile_fail
/// use openshell_ocsf::{EventContext, EventOrigin, NetworkActivityBuilder};
/// use openshell_ocsf::NetworkActivityBuilder;
///
/// let ctx = EventContext {
/// sandbox_id: String::new(),
/// sandbox_name: String::new(),
/// container_image: String::new(),
/// hostname: String::new(),
/// product_version: String::new(),
/// proxy_ip: "127.0.0.1".parse().unwrap(),
/// proxy_port: 3128,
/// origin: EventOrigin::Supervisor,
/// };
/// NetworkActivityBuilder::new(&ctx).build();
/// NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx()).build();
/// ```
#[must_use]
pub fn new(ctx: impl Into<ContextRef<'a>>) -> Self {
@@ -202,24 +192,6 @@ impl<'a, EndpointState> NetworkActivityBuilder<'a, EndpointState> {
self
}
#[must_use]
pub fn activity(mut self, id: ActivityId) -> Self {
self.activity = id;
self
}
#[must_use]
pub fn action(mut self, id: ActionId) -> Self {
self.action = Some(id);
self
}
#[must_use]
pub fn disposition(mut self, id: DispositionId) -> Self {
self.disposition = Some(id);
self
}
#[must_use]
pub fn actor_process(mut self, process: crate::objects::Process) -> Self {
self.actor = Some(Actor::process(process));
@@ -231,26 +203,12 @@ impl<'a, EndpointState> NetworkActivityBuilder<'a, EndpointState> {
self.firewall_rule = Some(FirewallRule::new(name, rule_type));
self
}
#[must_use]
pub fn severity(mut self, id: SeverityId) -> Self {
self.severity = id;
self
}
#[must_use]
pub fn status(mut self, id: StatusId) -> Self {
self.status = Some(id);
self
}
#[must_use]
pub fn message(mut self, msg: impl Into<String>) -> Self {
self.message = Some(msg.into());
self
}
}
impl_activity_setter!(NetworkActivityBuilder<EndpointState>);
impl_action_disposition_setters!(NetworkActivityBuilder<EndpointState>);
impl_builder_setters!(NetworkActivityBuilder<EndpointState>);
impl NetworkActivityBuilder<'_, HasNetworkEndpoint> {
/// Finalize and return the `OcsfEvent`.
#[must_use]
crates/openshell-ocsf/src/builders/process.rs
+6 -43
View File
@@ -37,19 +37,9 @@ impl<'a> ProcessActivityBuilder<'a, MissingActor> {
/// A Process Activity must name its acting process before it can be built.
///
/// ```compile_fail
/// use openshell_ocsf::{EventContext, EventOrigin, ProcessActivityBuilder};
/// use openshell_ocsf::ProcessActivityBuilder;
///
/// let ctx = EventContext {
/// sandbox_id: String::new(),
/// sandbox_name: String::new(),
/// container_image: String::new(),
/// hostname: String::new(),
/// product_version: String::new(),
/// proxy_ip: "127.0.0.1".parse().unwrap(),
/// proxy_port: 3128,
/// origin: EventOrigin::Supervisor,
/// };
/// ProcessActivityBuilder::new(&ctx).build();
/// ProcessActivityBuilder::new(openshell_ocsf::ctx::ctx()).build();
/// ```
#[must_use]
pub fn new(ctx: impl Into<ContextRef<'a>>) -> Self {
@@ -90,43 +80,12 @@ impl<'a> ProcessActivityBuilder<'a, MissingActor> {
}
impl<ActorState> ProcessActivityBuilder<'_, ActorState> {
/// Set the event activity identifier.
#[must_use]
pub fn activity(mut self, id: ActivityId) -> Self {
self.activity = id;
self
}
/// Set the action taken.
#[must_use]
pub fn action(mut self, id: ActionId) -> Self {
self.action = Some(id);
self
}
/// Set the disposition of the action.
#[must_use]
pub fn disposition(mut self, id: DispositionId) -> Self {
self.disposition = Some(id);
self
}
/// Set the event severity.
#[must_use]
pub fn severity(mut self, id: SeverityId) -> Self {
self.severity = id;
self
}
/// Set the overall event status.
#[must_use]
pub fn status(mut self, id: StatusId) -> Self {
self.status = Some(id);
self
}
/// Set a human-readable event message.
#[must_use]
pub fn message(mut self, msg: impl Into<String>) -> Self {
self.message = Some(msg.into());
self
}
#[must_use]
pub fn process(mut self, proc: Process) -> Self {
self.process = Some(proc);
self
@@ -143,6 +102,10 @@ impl<ActorState> ProcessActivityBuilder<'_, ActorState> {
}
}
impl_activity_setter!(ProcessActivityBuilder<ActorState>);
impl_action_disposition_setters!(ProcessActivityBuilder<ActorState>);
impl_builder_setters!(ProcessActivityBuilder<ActorState>);
impl ProcessActivityBuilder<'_, HasActor> {
#[must_use]
pub fn build(self) -> OcsfEvent {
crates/openshell-ocsf/src/builders/ssh.rs
+17 -74
View File
@@ -40,19 +40,9 @@ impl<'a> SshActivityBuilder<'a, MissingSshEndpoint> {
/// can be built.
///
/// ```compile_fail
/// use openshell_ocsf::{EventContext, EventOrigin, SshActivityBuilder};
/// use openshell_ocsf::SshActivityBuilder;
///
/// let ctx = EventContext {
/// sandbox_id: String::new(),
/// sandbox_name: String::new(),
/// container_image: String::new(),
/// hostname: String::new(),
/// product_version: String::new(),
/// proxy_ip: "127.0.0.1".parse().unwrap(),
/// proxy_port: 3128,
/// origin: EventOrigin::Supervisor,
/// };
/// SshActivityBuilder::new(&ctx).build();
/// SshActivityBuilder::new(openshell_ocsf::ctx::ctx()).build();
/// ```
#[must_use]
pub fn new(ctx: impl Into<ContextRef<'a>>) -> Self {
@@ -79,40 +69,31 @@ impl<'a, EndpointState> SshActivityBuilder<'a, EndpointState> {
/// Set the source endpoint from an address.
#[must_use]
pub fn src_endpoint_addr(
self,
mut self,
ip: std::net::IpAddr,
port: u16,
) -> SshActivityBuilder<'a, HasSshEndpoint> {
SshActivityBuilder {
src_endpoint: Some(Endpoint::from_ip(ip, port)),
dst_endpoint: self.dst_endpoint,
ctx: self.ctx,
activity: self.activity,
action: self.action,
disposition: self.disposition,
severity: self.severity,
status: self.status,
actor: self.actor,
auth_type_id: self.auth_type_id,
auth_type_label: self.auth_type_label,
protocol_ver: self.protocol_ver,
message: self.message,
endpoint_state: std::marker::PhantomData,
}
self.src_endpoint = Some(Endpoint::from_ip(ip, port));
self.with_endpoint()
}
/// Set the destination endpoint.
#[must_use]
pub fn dst_endpoint(self, endpoint: Endpoint) -> SshActivityBuilder<'a, HasSshEndpoint> {
pub fn dst_endpoint(mut self, endpoint: Endpoint) -> SshActivityBuilder<'a, HasSshEndpoint> {
self.dst_endpoint = Some(endpoint);
self.with_endpoint()
}
fn with_endpoint(self) -> SshActivityBuilder<'a, HasSshEndpoint> {
SshActivityBuilder {
src_endpoint: self.src_endpoint,
dst_endpoint: Some(endpoint),
ctx: self.ctx,
activity: self.activity,
action: self.action,
disposition: self.disposition,
severity: self.severity,
status: self.status,
src_endpoint: self.src_endpoint,
dst_endpoint: self.dst_endpoint,
actor: self.actor,
auth_type_id: self.auth_type_id,
auth_type_label: self.auth_type_label,
@@ -136,56 +117,18 @@ impl<'a, EndpointState> SshActivityBuilder<'a, EndpointState> {
self
}
/// Set the event activity identifier.
#[must_use]
pub fn activity(mut self, id: ActivityId) -> Self {
self.activity = id;
self
}
/// Set the action taken.
#[must_use]
pub fn action(mut self, id: ActionId) -> Self {
self.action = Some(id);
self
}
/// Set the disposition of the action.
#[must_use]
pub fn disposition(mut self, id: DispositionId) -> Self {
self.disposition = Some(id);
self
}
/// Set the acting process.
#[must_use]
pub fn actor_process(mut self, process: crate::objects::Process) -> Self {
self.actor = Some(Actor::process(process));
self
}
/// Set the event severity.
#[must_use]
pub fn severity(mut self, id: SeverityId) -> Self {
self.severity = id;
self
}
/// Set the overall event status.
#[must_use]
pub fn status(mut self, id: StatusId) -> Self {
self.status = Some(id);
self
}
/// Set a human-readable event message.
#[must_use]
pub fn message(mut self, msg: impl Into<String>) -> Self {
self.message = Some(msg.into());
self
}
}
impl_activity_setter!(SshActivityBuilder<EndpointState>);
impl_action_disposition_setters!(SshActivityBuilder<EndpointState>);
impl_builder_setters!(SshActivityBuilder<EndpointState>);
impl SshActivityBuilder<'_, HasSshEndpoint> {
#[must_use]
pub fn build(self) -> OcsfEvent {
crates/openshell-ocsf/src/ctx.rs
+13 -11
View File
@@ -16,18 +16,10 @@ use std::sync::{Arc, LazyLock, OnceLock};
static OCSF_CTX: OnceLock<EventContext> = OnceLock::new();
/// With `test-support`, producer tests get a production-like sandbox identity
/// so their events carry the same container and device fields production does.
#[cfg(any(test, feature = "test-support"))]
const FALLBACK_SANDBOX: (&str, &str, &str) =
("sandbox-test", "test-sandbox", "example/sandbox:test");
#[cfg(not(any(test, feature = "test-support")))]
const FALLBACK_SANDBOX: (&str, &str, &str) = ("", "", "");
static OCSF_CTX_FALLBACK: LazyLock<EventContext> = LazyLock::new(|| EventContext {
sandbox_id: FALLBACK_SANDBOX.0.to_string(),
sandbox_name: FALLBACK_SANDBOX.1.to_string(),
container_image: FALLBACK_SANDBOX.2.to_string(),
sandbox_id: String::new(),
sandbox_name: String::new(),
container_image: String::new(),
hostname: "test".to_string(),
product_version: env!("CARGO_PKG_VERSION").to_string(),
proxy_ip: std::net::IpAddr::from([127, 0, 0, 1]),
@@ -67,3 +59,13 @@ pub fn ctx() -> ContextRef<'static> {
}
ContextRef::Borrowed(OCSF_CTX.get().unwrap_or(&OCSF_CTX_FALLBACK))
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn fallback_context_names_no_sandbox() {
assert!(OCSF_CTX_FALLBACK.container().is_none());
}
}
docs/observability/ocsf-json-export.mdx
+2
View File
@@ -163,6 +163,8 @@ Each JSONL file line, or the payload after the stderr marker, is a complete OCSF
"category_name": "Network Activity",
"activity_id": 1,
"activity_name": "Open",
"type_uid": 400101,
"type_name": "Network Activity: Open",
"severity_id": 1,
"severity": "Informational",
"status_id": 1,
e2e/rust/tests/ocsf_jsonl_conformance.rs
+14 -15
View File
@@ -135,13 +135,7 @@ fn supervisor_records(engine: &ContainerEngine, sandbox: &str) -> Result<Vec<Val
let filter = format!("label=openshell.ai/sandbox-name={sandbox}");
let ps = engine
.command()
.args([
"ps",
"--filter",
&filter,
"--format",
"{{index .Labels \"openshell.ai/sandbox-id\"}}",
])
.args(["ps", "--filter", &filter, "--format", "json"])
.output()
.map_err(|e| format!("could not run ps: {e}"))?;
if !ps.status.success() {
@@ -150,11 +144,13 @@ fn supervisor_records(engine: &ContainerEngine, sandbox: &str) -> Result<Vec<Val
String::from_utf8_lossy(&ps.stderr)
));
}
let ids = String::from_utf8_lossy(&ps.stdout);
let id = ids
.lines()
.map(str::trim)
.find(|id| !id.is_empty())
// Podman 6 renders `.Labels` in a --format template as a string, so read
// the label from the JSON output, where it is an object in every version.
let containers: Vec<Value> =
serde_json::from_slice(&ps.stdout).map_err(|e| format!("parse ps output: {e}"))?;
let id = containers
.iter()
.find_map(|container| container["Labels"]["openshell.ai/sandbox-id"].as_str())
.ok_or_else(|| format!("no container matches {filter}"))?;
let dir = tempfile::tempdir().map_err(|e| format!("temp dir: {e}"))?;
@@ -222,8 +218,9 @@ async fn supervisor_jsonl_conforms_to_the_native_schema() {
// every required class appears.
let mut records = Vec::new();
let mut last_error = None;
let mut last_exec_error = None;
for _ in 0..12 {
let _ = guard.exec(&["python3", "-c", &traffic]).await;
last_exec_error = guard.exec(&["python3", "-c", &traffic]).await.err();
tokio::time::sleep(Duration::from_secs(5)).await;
match supervisor_records(&engine, &guard.name) {
Ok(found) => {
@@ -241,9 +238,11 @@ async fn supervisor_jsonl_conforms_to_the_native_schema() {
let missing = missing_classes(&records);
assert!(
missing.is_empty(),
"missing OCSF classes {missing:?} in {} records; last copy error: {}",
"missing OCSF classes {missing:?} in {} records; last copy error: {}; \
last exec error: {}",
records.len(),
last_error.as_deref().unwrap_or("none")
last_error.as_deref().unwrap_or("none"),
last_exec_error.as_deref().unwrap_or("none")
);
let mut failures = Vec::new();
for record in &records {