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
4 changes: 2 additions & 2 deletions .github/workflows/docker-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ jobs:
gateway)
binary_component=gateway
binary_name=openshell-gateway
features="openshell-core/dev-settings bundled-z3"
features="bundled-z3"
has_image=true
;;
supervisor)
binary_component=sandbox
binary_name=openshell-sandbox
features="openshell-core/dev-settings"
features=""
has_image=true
;;
cli)
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/rust-native-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ on:
description: "Cargo features to enable"
required: false
type: string
default: "openshell-core/dev-settings"
default: ""
Comment thread
SDAChess marked this conversation as resolved.
retention-days:
description: "Artifact retention period"
required: false
Expand Down
1 change: 0 additions & 1 deletion crates/openshell-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ workspace = true

[features]
bundled-z3 = ["openshell-prover/bundled-z3"]
dev-settings = ["openshell-core/dev-settings"]

[dev-dependencies]
futures = { workspace = true }
Expand Down
20 changes: 4 additions & 16 deletions crates/openshell-cli/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8097,16 +8097,15 @@ mod tests {
));
}

#[cfg(feature = "dev-settings")]
#[test]
fn parse_cli_setting_value_parses_bool_aliases() {
let yes_value = parse_cli_setting_value("dummy_bool", "yes").expect("parse yes");
let yes_value = parse_cli_setting_value("ocsf_json_enabled", "yes").expect("parse yes");
Comment thread
elezar marked this conversation as resolved.
assert_eq!(
yes_value.value,
Some(openshell_core::proto::setting_value::Value::BoolValue(true))
);

let zero_value = parse_cli_setting_value("dummy_bool", "0").expect("parse 0");
let zero_value = parse_cli_setting_value("ocsf_json_enabled", "0").expect("parse 0");
assert_eq!(
zero_value.value,
Some(openshell_core::proto::setting_value::Value::BoolValue(
Expand All @@ -8115,21 +8114,10 @@ mod tests {
);
}

#[cfg(feature = "dev-settings")]
#[test]
fn parse_cli_setting_value_parses_int_key() {
let int_value = parse_cli_setting_value("dummy_int", "42").expect("parse int");
Comment thread
elezar marked this conversation as resolved.
assert_eq!(
int_value.value,
Some(openshell_core::proto::setting_value::Value::IntValue(42))
);
}

#[cfg(feature = "dev-settings")]
#[test]
fn parse_cli_setting_value_rejects_invalid_bool() {
let err =
parse_cli_setting_value("dummy_bool", "maybe").expect_err("invalid bool should fail");
let err = parse_cli_setting_value("ocsf_json_enabled", "maybe")
.expect_err("invalid bool should fail");
assert!(err.to_string().contains("invalid bool value"));
}

Expand Down
4 changes: 0 additions & 4 deletions crates/openshell-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ default = ["telemetry"]
## `--no-default-features` (plus any other features you need) for a build that
## contains no telemetry endpoint, no HTTP client, and no emission code at all.
telemetry = ["dep:reqwest", "dep:chrono"]
## Include test-only settings (dummy_bool, dummy_int) in the registry.
## Off by default so production builds have an empty registry.
## Enabled by e2e tests and during development.
dev-settings = []
## Expose proposals::test_helpers (`ProposalsFlagGuard`) to downstream test
## code in other crates. Enabled by openshell-sandbox and
## openshell-supervisor-network dev builds.
Expand Down
23 changes: 0 additions & 23 deletions crates/openshell-core/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,20 +137,6 @@ pub const REGISTERED_SETTINGS: &[RegisteredSetting] = &[
kind: SettingValueKind::String,
allowed_string_values: Some(PROPOSAL_APPROVAL_MODE_VALUES),
},
// Test-only keys live behind the `dev-settings` feature flag so they
// don't appear in production builds.
#[cfg(feature = "dev-settings")]
RegisteredSetting {
key: "dummy_int",
kind: SettingValueKind::Int,
allowed_string_values: None,
},
#[cfg(feature = "dev-settings")]
RegisteredSetting {
key: "dummy_bool",
kind: SettingValueKind::Bool,
allowed_string_values: None,
},
];

/// Resolve a setting descriptor from the registry by key.
Expand Down Expand Up @@ -187,15 +173,6 @@ mod tests {
registered_keys_csv, setting_for_key,
};

#[cfg(feature = "dev-settings")]
#[test]
fn setting_for_key_returns_dev_entries() {
let setting = setting_for_key("dummy_bool").expect("dummy_bool should be registered");
assert_eq!(setting.kind, SettingValueKind::Bool);
let setting = setting_for_key("dummy_int").expect("dummy_int should be registered");
assert_eq!(setting.kind, SettingValueKind::Int);
}

#[test]
fn setting_for_key_returns_none_for_unknown() {
assert!(setting_for_key("nonexistent_key").is_none());
Expand Down
1 change: 0 additions & 1 deletion crates/openshell-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ default = ["telemetry"]
## that contains no telemetry endpoint, HTTP client, or emission code.
telemetry = ["openshell-core/telemetry"]
bundled-z3 = ["openshell-prover/bundled-z3"]
dev-settings = ["openshell-core/dev-settings"]
test-support = []

[dev-dependencies]
Expand Down
94 changes: 53 additions & 41 deletions crates/openshell-server/src/grpc/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8895,24 +8895,22 @@ mod tests {
assert!(err.message().contains("unknown setting key"));
}

#[cfg(feature = "dev-settings")]
#[test]
fn proto_setting_to_stored_rejects_type_mismatch() {
let value = SettingValue {
value: Some(setting_value::Value::StringValue("true".to_string())),
};
let err = proto_setting_to_stored("dummy_bool", &value).unwrap_err();
let err = proto_setting_to_stored("ocsf_json_enabled", &value).unwrap_err();
assert_eq!(err.code(), Code::InvalidArgument);
assert!(err.message().contains("expects bool value"));
}

#[cfg(feature = "dev-settings")]
#[test]
fn proto_setting_to_stored_accepts_bool_for_registered_bool_key() {
let value = SettingValue {
value: Some(setting_value::Value::BoolValue(true)),
};
let stored = proto_setting_to_stored("dummy_bool", &value).unwrap();
let stored = proto_setting_to_stored("ocsf_json_enabled", &value).unwrap();
assert_eq!(stored, StoredSettingValue::Bool(true));
}

Expand Down Expand Up @@ -8988,17 +8986,19 @@ mod tests {
);
}

#[cfg(feature = "dev-settings")]
#[test]
fn merge_effective_settings_global_overrides_sandbox_key() {
let global = StoredSettings {
revision: 2,
settings: [
(
"log_level".to_string(),
StoredSettingValue::String("warn".to_string()),
settings::PROVIDERS_V2_ENABLED_KEY.to_string(),
StoredSettingValue::Bool(false),
),
(
settings::AGENT_POLICY_PROPOSALS_ENABLED_KEY.to_string(),
StoredSettingValue::Bool(false),
),
("dummy_int".to_string(), StoredSettingValue::Int(7)),
]
.into_iter()
.collect(),
Expand All @@ -9008,50 +9008,59 @@ mod tests {
revision: 1,
settings: [
(
"log_level".to_string(),
StoredSettingValue::String("debug".to_string()),
settings::PROVIDERS_V2_ENABLED_KEY.to_string(),
StoredSettingValue::Bool(true),
),
(
"ocsf_json_enabled".to_string(),
StoredSettingValue::Bool(true),
),
("dummy_bool".to_string(), StoredSettingValue::Bool(true)),
]
.into_iter()
.collect(),
..Default::default()
};

let merged = merge_effective_settings(&global, &sandbox).unwrap();
let log_level = merged.get("log_level").expect("log_level present");
assert_eq!(log_level.scope, SettingScope::Global as i32);
let providers_v2 = merged
.get(settings::PROVIDERS_V2_ENABLED_KEY)
.expect("providers_v2_enabled present");
assert_eq!(providers_v2.scope, SettingScope::Global as i32);
assert_eq!(
log_level.value.as_ref().and_then(|v| v.value.as_ref()),
Some(&setting_value::Value::StringValue("warn".to_string()))
providers_v2.value.as_ref().and_then(|v| v.value.as_ref()),
Some(&setting_value::Value::BoolValue(false))
);

let dummy_bool = merged.get("dummy_bool").expect("dummy_bool present");
assert_eq!(dummy_bool.scope, SettingScope::Sandbox as i32);
let ocsf_json = merged
.get("ocsf_json_enabled")
.expect("ocsf_json_enabled present");
assert_eq!(ocsf_json.scope, SettingScope::Sandbox as i32);

let dummy_int = merged.get("dummy_int").expect("dummy_int present");
assert_eq!(dummy_int.scope, SettingScope::Global as i32);
let proposals = merged
.get(settings::AGENT_POLICY_PROPOSALS_ENABLED_KEY)
.expect("agent_policy_proposals_enabled present");
assert_eq!(proposals.scope, SettingScope::Global as i32);
}

#[cfg(feature = "dev-settings")]
#[test]
fn merge_effective_settings_sandbox_scoped_value_has_sandbox_scope() {
let global = StoredSettings::default();
let sandbox = StoredSettings {
revision: 1,
settings: [(
"log_level".to_string(),
StoredSettingValue::String("debug".to_string()),
)]
.into_iter()
settings: std::iter::once((
"ocsf_json_enabled".to_string(),
StoredSettingValue::Bool(true),
))
.collect(),
..Default::default()
};

let merged = merge_effective_settings(&global, &sandbox).unwrap();
let log_level = merged.get("log_level").expect("log_level present");
assert_eq!(log_level.scope, SettingScope::Sandbox as i32);
assert!(log_level.value.is_some());
let ocsf_json = merged
.get("ocsf_json_enabled")
.expect("ocsf_json_enabled present");
assert_eq!(ocsf_json.scope, SettingScope::Sandbox as i32);
assert!(ocsf_json.value.is_some());
}

#[test]
Expand Down Expand Up @@ -9274,9 +9283,10 @@ mod tests {
"log_level".to_string(),
StoredSettingValue::String("error".to_string()),
);
settings
.settings
.insert("dummy_bool".to_string(), StoredSettingValue::Bool(true));
settings.settings.insert(
"ocsf_json_enabled".to_string(),
StoredSettingValue::Bool(true),
);
settings.revision = 5;
save_global_settings(&store, &settings).await.unwrap();

Expand All @@ -9287,7 +9297,7 @@ mod tests {
Some(&StoredSettingValue::String("error".to_string()))
);
assert_eq!(
loaded.settings.get("dummy_bool"),
loaded.settings.get("ocsf_json_enabled"),
Some(&StoredSettingValue::Bool(true))
);
}
Expand All @@ -9298,9 +9308,10 @@ mod tests {

let sandbox_name = "my-sandbox";
let mut settings = StoredSettings::default();
settings
.settings
.insert("dummy_int".to_string(), StoredSettingValue::Int(99));
settings.settings.insert(
settings::PROPOSAL_APPROVAL_MODE_KEY.to_string(),
StoredSettingValue::String("auto".to_string()),
);
settings.revision = 3;
save_sandbox_settings(&store, sandbox_name, &settings)
.await
Expand All @@ -9309,8 +9320,8 @@ mod tests {
let loaded = load_sandbox_settings(&store, sandbox_name).await.unwrap();
assert_eq!(loaded.revision, 3);
assert_eq!(
loaded.settings.get("dummy_int"),
Some(&StoredSettingValue::Int(99))
loaded.settings.get(settings::PROPOSAL_APPROVAL_MODE_KEY),
Some(&StoredSettingValue::String("auto".to_string()))
);
}

Expand Down Expand Up @@ -9434,14 +9445,15 @@ mod tests {
let store = test_store().await;

let mut global = StoredSettings::default();
global
.settings
.insert("dummy_int".to_string(), StoredSettingValue::Int(42));
global.settings.insert(
"ocsf_json_enabled".to_string(),
StoredSettingValue::Bool(true),
);
global.revision = 1;
save_global_settings(&store, &global).await.unwrap();

let loaded_global = load_global_settings(&store).await.unwrap();
assert!(loaded_global.settings.contains_key("dummy_int"));
assert!(loaded_global.settings.contains_key("ocsf_json_enabled"));
}

#[tokio::test]
Expand Down
2 changes: 1 addition & 1 deletion e2e/rust/e2e-docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
E2E_TEST="${OPENSHELL_E2E_DOCKER_TEST:-smoke}"
E2E_FEATURES="${OPENSHELL_E2E_DOCKER_FEATURES:-e2e,e2e-docker}"

cargo build -p openshell-cli --features openshell-core/dev-settings
cargo build -p openshell-cli

exec "${ROOT}/e2e/with-docker-gateway.sh" \
cargo test --manifest-path "${ROOT}/e2e/rust/Cargo.toml" \
Expand Down
2 changes: 1 addition & 1 deletion e2e/rust/e2e-kubernetes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"

E2E_FEATURES="${OPENSHELL_E2E_KUBERNETES_FEATURES:-e2e,e2e-host-gateway,e2e-kubernetes}"

cargo build -p openshell-cli --features openshell-core/dev-settings
cargo build -p openshell-cli

test_filter=()
if [ -n "${OPENSHELL_E2E_KUBE_TEST:-}" ]; then
Expand Down
2 changes: 1 addition & 1 deletion e2e/rust/e2e-podman.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
E2E_TEST="${OPENSHELL_E2E_PODMAN_TEST:-}"
E2E_FEATURES="${OPENSHELL_E2E_PODMAN_FEATURES:-e2e-podman}"

cargo build -p openshell-cli --features openshell-core/dev-settings
cargo build -p openshell-cli

TEST_ARGS=(
cargo test --manifest-path "${ROOT}/e2e/rust/Cargo.toml"
Expand Down
3 changes: 1 addition & 2 deletions e2e/rust/e2e-vm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ echo "==> Building openshell-gateway, openshell-driver-vm, openshell (CLI)"
cargo build \
-p openshell-server \
-p openshell-driver-vm \
-p openshell-cli \
--features openshell-core/dev-settings
-p openshell-cli

if [ "$(uname -s)" = "Darwin" ]; then
echo "==> Codesigning openshell-driver-vm (Hypervisor entitlement)"
Expand Down
6 changes: 3 additions & 3 deletions e2e/rust/tests/settings_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use openshell_e2e::harness::output::strip_ansi;
use openshell_e2e::harness::sandbox::SandboxGuard;
use tokio::time::{Instant, sleep};

const TEST_KEY: &str = "dummy_bool";
const TEST_KEY: &str = "ocsf_json_enabled";
static SETTINGS_E2E_LOCK: Mutex<()> = Mutex::new(());

struct CliResult {
Expand Down Expand Up @@ -231,7 +231,7 @@ async fn settings_global_override_round_trip() {
assert!(
set_global
.clean_output
.contains("Set global setting dummy_bool=false"),
.contains(&format!("Set global setting {TEST_KEY}=false")),
"expected global set output:\n{}",
set_global.clean_output
);
Expand Down Expand Up @@ -289,7 +289,7 @@ async fn settings_global_override_round_trip() {
assert!(
delete_global
.clean_output
.contains("Deleted global setting dummy_bool"),
.contains(&format!("Deleted global setting {TEST_KEY}")),
"expected global delete confirmation in output:\n{}",
delete_global.clean_output
);
Expand Down
Loading
Loading