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: 3 additions & 1 deletion crates/omnigraph-cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ control — manage or inspect a cluster (cluster via --config; policy & queries
--cluster).\n \
local — no explicit graph scope; local config & tooling: alias, embed, login, logout, profile, version.\n\
MANAGED FOLDERS: cluster commands use .omnigraph/context; cluster token caches data access.\n\
query and mutate require --graph and a cached data credential. --direct selects legacy addressing.\n\
Implicit query and mutate use folder context and require --graph plus a cached data credential.\n\
Explicit target selectors retain ordinary addressing; competing ambient targets refuse.\n\
--direct selects ordinary addressing, including operator profiles and defaults.\n\
See the 'Command capabilities' section of the CLI reference for which flags apply where.")]
pub(crate) struct Cli {
/// Explicitly use legacy addressing and credentials, ignoring folder context.
Expand Down
49 changes: 37 additions & 12 deletions crates/omnigraph-cli/src/managed/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ fn scope(cli: &Cli) -> Result<()> {
{
return Err(Failure::refused(
"managed_scope_conflict",
"managed data uses folder context and its cached credential; --server, --profile, --store, --cluster, and --as require explicit --direct",
"this managed operation uses its selected context; ordinary target selectors and an explicit --as are not applicable",
));
}
Ok(())
Expand Down Expand Up @@ -317,18 +317,48 @@ fn load(

fn skips_context(cli: &Cli) -> bool {
cli.direct
|| matches!(cli.command, Command::Cluster { .. })
|| (crate::planes::command_plane(&cli.command) == crate::planes::Plane::Session
&& !matches!(cli.command, Command::Alias { .. }))
|| !matches!(cli.command, Command::Query { .. } | Command::Mutate { .. })
|| cli.server.is_some()
|| cli.profile.is_some()
|| cli.store.is_some()
|| cli.cluster.is_some()
}

/// This check never resolves a competing target or consults credentials. Even
/// an unknown profile or matching-looking server URL is ambiguous beside a
/// managed binding. Call it only after selecting a valid exact-directory
/// context, so ordinary commands retain their own operator-config behavior.
fn has_ambient_target() -> Result<bool> {
if std::env::var_os(crate::scope::PROFILE_ENV).is_some_and(|value| !value.is_empty()) {
return Ok(true);
}
let operator = crate::operator::load_operator_config().map_err(|_| {
Failure::refused(
"operator_config_invalid",
"cannot read valid operator configuration to exclude a competing data target",
)
})?;
Ok(operator.default_server().is_some() || operator.default_store().is_some())
}

fn resolve(cli: &Cli, config: &std::path::Path, store: &impl Store) -> Result<Option<GraphClient>> {
fn resolve(
cli: &Cli,
config: &std::path::Path,
store: &impl Store,
ambient_target: impl FnOnce() -> Result<bool>,
) -> Result<Option<GraphClient>> {
if skips_context(cli) {
return Ok(None);
}
let Some(context) = super::read_context(config)? else {
return Ok(None);
};
if ambient_target()? {
return Err(Failure::refused(
"managed_target_ambiguous",
"folder context competes with OMNIGRAPH_PROFILE or an operator default target; select the intended ordinary target explicitly, use --direct for ordinary ambient resolution, or clear the competing ambient target to use this managed folder",
));
}
let (action, named) = match &cli.command {
Command::Query {
query,
Expand All @@ -340,12 +370,7 @@ fn resolve(cli: &Cli, config: &std::path::Path, store: &impl Store) -> Result<Op
query_string,
..
} => ("change", query.is_none() && query_string.is_none()),
_ => {
return Err(Failure::refused(
"managed_command_unsupported",
"managed data currently supports query and mutate; use --direct only when legacy addressing is intended",
));
}
_ => unreachable!("only implicit query/mutate consult data context"),
};
scope(cli)?;
let graph = cli
Expand Down Expand Up @@ -374,7 +399,7 @@ pub(crate) fn client(cli: &Cli) -> std::result::Result<Option<GraphClient>, Outp
};
let result = std::env::current_dir()
.map_err(|_| Failure::refused("context_invalid", "cannot resolve the current directory"))
.and_then(|cwd| resolve(cli, &cwd, &auth::DATA_STORE));
.and_then(|cwd| resolve(cli, &cwd, &auth::DATA_STORE, has_ambient_target));
result.map_err(|e| Output::from_result(Err(e), json, 2))
}

Expand Down
137 changes: 119 additions & 18 deletions crates/omnigraph-cli/src/managed/data/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,28 +355,19 @@ async fn invalid_issuance_never_replaces_cached_authority() {
}

#[test]
fn managed_routing_refuses_scope_overrides_and_other_data_verbs_before_keychain() {
fn managed_routing_preserves_selected_managed_authority_without_fallback() {
let dir = tempfile::tempdir().unwrap();
let context = context();
super::super::save_context(dir.path(), &context).unwrap();
let store = MemoryStore::default();
for args in [
vec!["query", "q"],
vec![
"query",
"q",
"--graph",
"knowledge",
"--server",
"https://foreign.example",
],
vec!["mutate", "m", "--graph", "knowledge", "--as", "fake"],
vec!["snapshot", "--store", "/tmp/never-open"],
vec!["alias", "dangerous"],
vec!["graphs", "list"],
] {
let cli = Cli::try_parse_from(std::iter::once("omnigraph").chain(args)).unwrap();
let failure = resolve(&cli, dir.path(), &store).err().unwrap();
let failure = resolve(&cli, dir.path(), &store, || Ok(false))
.err()
.unwrap();
assert_ne!(failure.body["type"], "data_credential_required");
}
let direct = Cli::try_parse_from([
Expand All @@ -388,27 +379,137 @@ fn managed_routing_refuses_scope_overrides_and_other_data_verbs_before_keychain(
"https://legacy.example",
])
.unwrap();
assert!(resolve(&direct, dir.path(), &store).unwrap().is_none());
assert!(
resolve(&direct, dir.path(), &store, || panic!(
"explicit selection read operator defaults"
))
.unwrap()
.is_none()
);
let child = dir.path().join("child");
std::fs::create_dir(&child).unwrap();
let query = Cli::try_parse_from(["omnigraph", "query", "q", "--graph", "knowledge"]).unwrap();
assert!(resolve(&query, &child, &store).unwrap().is_none());
assert!(
resolve(&query, &child, &store, || panic!(
"absent context read operator defaults"
))
.unwrap()
.is_none()
);
assert_eq!(
resolve(&query, dir.path(), &store).err().unwrap().body["type"],
resolve(&query, dir.path(), &store, || Ok(false))
.err()
.unwrap()
.body["type"],
"data_credential_required"
);
let cached = credential(&context, "https://data.example");
save(&store, &context, &cached);
assert!(resolve(&query, dir.path(), &store).unwrap().is_some());
assert!(
resolve(&query, dir.path(), &store, || Ok(false))
.unwrap()
.is_some()
);
let mut narrowed = cached;
narrowed.grants[0].actions = vec!["read".into()];
save(&store, &context, &narrowed);
assert_eq!(
resolve(&query, dir.path(), &store).err().unwrap().body["type"],
resolve(&query, dir.path(), &store, || Ok(false))
.err()
.unwrap()
.body["type"],
"data_scope_missing"
);
}

struct NoCredentialAccess;

impl Store for NoCredentialAccess {
fn get(&self, _: &str) -> Result<Option<String>> {
panic!("routing preflight accessed credentials")
}
fn put(&self, _: &str, _: &str) -> Result<()> {
panic!("routing preflight wrote credentials")
}
fn remove(&self, _: &str) -> Result<()> {
panic!("routing preflight removed credentials")
}
}

#[test]
fn managed_data_issue_633_explicit_and_unrelated_commands_skip_context() {
let dir = tempfile::tempdir().unwrap();
super::super::save_context(dir.path(), &context()).unwrap();
// An unreadable-as-context object makes an accidental context read fail;
// the ambient callback and store also fail if either is consulted.
std::fs::remove_file(dir.path().join(".omnigraph/context")).unwrap();
std::fs::create_dir(dir.path().join(".omnigraph/context")).unwrap();
for args in [
vec!["query", "q", "--server", "legacy"],
vec!["read", "q", "--profile", "legacy"],
vec!["mutate", "--store", "file:///scratch", "-e", "source"],
vec!["change", "m", "--cluster", "local"],
vec!["query", "q", "--direct"],
vec!["init", "--schema", "schema.pg", "file:///scratch"],
vec!["load", "--data", "data.jsonl", "--mode", "append"],
vec!["schema", "plan", "--schema", "schema.pg"],
vec!["commit", "list"],
vec!["graphs", "list"],
vec!["alias", "people"],
vec!["queries", "list"],
vec!["queries", "validate"],
vec!["lint", "--schema", "schema.pg", "--query", "q.gq"],
vec!["snapshot"],
vec!["branch", "list"],
vec!["cluster", "status"],
] {
let cli = Cli::try_parse_from(std::iter::once("omnigraph").chain(args)).unwrap();
assert!(
resolve(&cli, dir.path(), &NoCredentialAccess, || {
panic!("bypassed command read operator routing")
})
.unwrap()
.is_none()
);
}
}

#[test]
fn managed_data_issue_633_ambiguity_and_invalid_context_precede_credentials() {
let dir = tempfile::tempdir().unwrap();
super::super::save_context(dir.path(), &context()).unwrap();
for verb in ["query", "read", "mutate", "change"] {
let cli = Cli::try_parse_from(["omnigraph", verb, "q", "--graph", "knowledge"]).unwrap();
assert_eq!(
resolve(&cli, dir.path(), &NoCredentialAccess, || Ok(true))
.err()
.unwrap()
.body["type"],
"managed_target_ambiguous"
);
assert_eq!(
resolve(&cli, dir.path(), &NoCredentialAccess, || {
Err(Failure::refused("operator_config_invalid", "fixture"))
})
.err()
.unwrap()
.body["type"],
"operator_config_invalid"
);
}
std::fs::write(dir.path().join(".omnigraph/context"), "malformed").unwrap();
let cli = Cli::try_parse_from(["omnigraph", "query", "q", "--graph", "knowledge"]).unwrap();
assert_eq!(
resolve(&cli, dir.path(), &NoCredentialAccess, || {
panic!("invalid context consulted another target")
})
.err()
.unwrap()
.body["type"],
"context_invalid"
);
}

#[tokio::test]
async fn managed_data_transport_refuses_redirect_and_bounds_body() {
let target = IntentApiFixture::new(vec![]);
Expand Down
Loading