From a819ab500e3c4e652089c27c40784e9e02e60fde Mon Sep 17 00:00:00 2001 From: aaltshuler Date: Thu, 11 Jun 2026 21:24:51 +0300 Subject: [PATCH] =?UTF-8?q?feat(cli):=20keyed=20credentials=20=E2=80=94=20?= =?UTF-8?q?servers:,=20the=20token=20chain,=20login/logout=20(RFC-007=20PR?= =?UTF-8?q?=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The operator config gains servers: (name -> url; never a token). A remote command whose URL prefix-matches an operator server resolves its bearer token through the keyed chain first — OMNIGRAPH_TOKEN_ env, then the [] section of ~/.omnigraph/credentials (created 0600 via temp+rename, #139 finding 7; group/world-readable files refused loudly) — falling through to the legacy chain unchanged. URL keying makes §D5 rule 3 structural: a token is only ever sent to the server it is keyed to. Longest-prefix matching with a path-boundary check (http://h:8080 never matches http://h:8080-evil). Inserting the keyed hop above the legacy chain is safe by construction — no existing setup can have servers: defined. omnigraph login stores/rotates one section (token from --token or one stdin line — the pipe flow keeps secrets out of shell history); omnigraph logout removes it, idempotently; logging in before declaring the server warns instead of failing (the gh model). Coverage: URL-match/no-substring-trap, credentials round-trip preserving sibling sections, 0600 write + over-permissive refusal, env-name mapping; the legacy resolve test is now hermetic against a real ~/.omnigraph and asserts byte-identical legacy behavior with no servers defined; one spawned-binary e2e walks the whole lifecycle against an authed server: refusal -> wrong-token login (stdin) -> rotate (--token) -> authorized read -> env-beats-file -> non-matching-URL negative -> logout revokes. Co-Authored-By: Claude Fable 5 --- crates/omnigraph-cli/src/cli.rs | 22 ++ crates/omnigraph-cli/src/helpers.rs | 31 ++ crates/omnigraph-cli/src/main.rs | 23 ++ crates/omnigraph-cli/src/main_tests.rs | 11 + crates/omnigraph-cli/src/operator.rs | 324 ++++++++++++++++++++- crates/omnigraph-cli/src/output.rs | 45 +++ crates/omnigraph-cli/tests/support/mod.rs | 9 + crates/omnigraph-cli/tests/system_local.rs | 115 ++++++++ docs/dev/rfc-007-operator-config.md | 4 +- docs/user/cli-reference.md | 23 ++ 10 files changed, 603 insertions(+), 4 deletions(-) diff --git a/crates/omnigraph-cli/src/cli.rs b/crates/omnigraph-cli/src/cli.rs index 6b59559..7708c0a 100644 --- a/crates/omnigraph-cli/src/cli.rs +++ b/crates/omnigraph-cli/src/cli.rs @@ -29,6 +29,28 @@ pub(crate) struct Cli { pub(crate) enum Command { /// Print the CLI version Version, + /// Store a bearer token for a named server in ~/.omnigraph/credentials + /// (0600). Token from --token or one line on stdin: + /// `echo $TOKEN | omnigraph login prod`. The keyed token applies to + /// requests whose URL matches the server's `url` in the operator + /// config's `servers:` map. + Login { + /// Server name (keys the credential; declare its url under + /// `servers:` in ~/.omnigraph/config.yaml) + name: String, + /// The token. Prefer piping via stdin over this flag (shell + /// history). + #[arg(long)] + token: Option, + #[arg(long)] + json: bool, + }, + /// Remove a named server's stored credential. Idempotent. + Logout { + name: String, + #[arg(long)] + json: bool, + }, /// Generate, clean, or refresh explicit seed embeddings Embed(EmbedArgs), /// Initialize a new graph from a schema diff --git a/crates/omnigraph-cli/src/helpers.rs b/crates/omnigraph-cli/src/helpers.rs index 85eb42a..b837192 100644 --- a/crates/omnigraph-cli/src/helpers.rs +++ b/crates/omnigraph-cli/src/helpers.rs @@ -221,6 +221,21 @@ pub(crate) fn resolve_remote_bearer_token( explicit_uri: Option<&str>, explicit_target: Option<&str>, ) -> Result> { + // The keyed hop (RFC-007 §D4, gh-host model): when the effective remote + // URL belongs to an operator-defined server, that server's keyed chain + // applies first — OMNIGRAPH_TOKEN_ env, then the 0600 credentials + // file. Ok(None) falls through to the legacy chain unchanged, and the + // keyed token is structurally scoped to its own server (§D5 rule 3): + // a URL matching no operator server never sees it. + if let Some(remote_url) = effective_remote_url(config, explicit_uri, explicit_target) { + let operator_config = operator::load_operator_config()?; + if let Some(server) = operator_config.find_server_for_url(&remote_url) { + if let Some(token) = operator::resolve_keyed_token(server)? { + return Ok(Some(token)); + } + } + } + let scoped_env = config.graph_bearer_token_env(explicit_uri, explicit_target, config.cli_graph_name()); let mut env_names = Vec::new(); @@ -249,6 +264,22 @@ pub(crate) fn resolve_remote_bearer_token( Ok(None) } +/// The remote base URL a token resolution is FOR — the same scoping +/// `graph_bearer_token_env` uses: an explicit http(s) `--uri` wins, else +/// the config-resolved target's uri (when remote). Local URIs → None. +fn effective_remote_url( + config: &OmnigraphConfig, + explicit_uri: Option<&str>, + explicit_target: Option<&str>, +) -> Option { + if let Some(uri) = explicit_uri { + return is_remote_uri(uri).then(|| uri.to_string()); + } + let target = config.resolve_target_name(explicit_uri, explicit_target, config.cli_graph_name())?; + let uri = &config.graphs.get(target)?.uri; + is_remote_uri(uri).then(|| uri.clone()) +} + pub(crate) fn build_http_client() -> Result { Ok(reqwest::Client::new()) } diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index bef111f..85fe537 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -73,6 +73,29 @@ async fn main() -> Result<()> { }; let http_client = build_http_client()?; match cli.command { + Command::Login { name, token, json } => { + let token = match token { + Some(token) => token, + None => { + let mut line = String::new(); + std::io::stdin().read_line(&mut line)?; + line + } + }; + let Some(token) = normalize_bearer_token(Some(token)) else { + color_eyre::eyre::bail!( + "no token provided: pass --token or pipe it on stdin (echo $TOKEN | omnigraph login {name})" + ); + }; + let operator_config = crate::operator::load_operator_config()?; + let declared = operator_config.servers.contains_key(&name); + let path = crate::operator::write_credential(&name, &token)?; + finish_login(&name, &path, declared, json)?; + } + Command::Logout { name, json } => { + let path = crate::operator::remove_credential(&name)?; + finish_logout(&name, &path, json)?; + } Command::Version => { println!("omnigraph {}", env!("CARGO_PKG_VERSION")); } diff --git a/crates/omnigraph-cli/src/main_tests.rs b/crates/omnigraph-cli/src/main_tests.rs index 0bbb593..8380c36 100644 --- a/crates/omnigraph-cli/src/main_tests.rs +++ b/crates/omnigraph-cli/src/main_tests.rs @@ -195,8 +195,14 @@ cli: .unwrap(); let previous = std::env::var_os(DEFAULT_BEARER_TOKEN_ENV); + let previous_home = std::env::var_os("OMNIGRAPH_HOME"); unsafe { std::env::remove_var(DEFAULT_BEARER_TOKEN_ENV); + // Hermetic: the keyed hop (RFC-007 PR 2) must not pick up a real + // ~/.omnigraph on the developer's machine — and with no operator + // servers defined, the legacy chain below must behave + // byte-identically to pre-PR-2 (tested-as-untouched). + std::env::set_var("OMNIGRAPH_HOME", temp.path().join("no-operator-config")); } let config_path = temp.path().join("omnigraph.yaml"); @@ -221,6 +227,11 @@ cli: } else { std::env::remove_var(DEFAULT_BEARER_TOKEN_ENV); } + if let Some(value) = previous_home { + std::env::set_var("OMNIGRAPH_HOME", value); + } else { + std::env::remove_var("OMNIGRAPH_HOME"); + } } } diff --git a/crates/omnigraph-cli/src/operator.rs b/crates/omnigraph-cli/src/operator.rs index bac37b3..1b95e24 100644 --- a/crates/omnigraph-cli/src/operator.rs +++ b/crates/omnigraph-cli/src/operator.rs @@ -13,6 +13,7 @@ //! config (server-side identity comes from bearer auth — invariant 11 //! holds by construction). +use std::collections::BTreeMap; use std::env; use std::path::{Path, PathBuf}; @@ -32,12 +33,24 @@ pub(crate) struct OperatorConfig { pub(crate) operator: OperatorIdentity, #[serde(default)] pub(crate) defaults: OperatorDefaults, + /// Operator-owned endpoint definitions (RFC-007 §D2/§D4): name → url. + /// The name keys the credential chain; nothing a repo checkout supplies + /// can redefine an entry here. No tokens in this file, ever. + #[serde(default)] + pub(crate) servers: BTreeMap, /// Everything this CLI version doesn't know. Warned once at load, /// otherwise ignored (forward compatibility within the operator layer). #[serde(flatten)] unknown: serde_yaml::Mapping, } +#[derive(Debug, Deserialize)] +pub(crate) struct OperatorServer { + pub(crate) url: String, + #[serde(flatten)] + unknown: serde_yaml::Mapping, +} + #[derive(Debug, Default, Deserialize)] pub(crate) struct OperatorIdentity { /// Default actor for every `--as` cascade (CLI direct-engine writes and @@ -64,6 +77,26 @@ impl OperatorConfig { pub(crate) fn output(&self) -> Option { self.defaults.output } + + /// The gh-host model: which operator server (if any) does this request + /// URL belong to? Longest-prefix match after trailing-slash + /// normalization, so `url: http://h:8080` matches + /// `http://h:8080/graphs/spike` but never `http://h:8080-evil`. + pub(crate) fn find_server_for_url(&self, request_url: &str) -> Option<&str> { + let request = request_url.trim_end_matches('/'); + let mut best: Option<(&str, usize)> = None; + for (name, server) in &self.servers { + let base = server.url.trim_end_matches('/'); + let matches = request == base + || request + .strip_prefix(base) + .is_some_and(|rest| rest.starts_with('/')); + if matches && best.is_none_or(|(_, len)| base.len() > len) { + best = Some((name, base.len())); + } + } + best.map(|(name, _)| name) + } } /// The operator dir: `$OMNIGRAPH_HOME` if set (tilde-expanded), else @@ -127,10 +160,216 @@ impl OperatorConfig { collect(&self.unknown, ""); collect(&self.operator.unknown, "operator."); collect(&self.defaults.unknown, "defaults."); + for (name, server) in &self.servers { + collect(&server.unknown, &format!("servers.{name}.")); + } warnings } } +// ---- keyed credentials (RFC-007 §D4) ---- + +pub(crate) const CREDENTIALS_FILE: &str = "credentials"; +const TOKEN_ENV_PREFIX: &str = "OMNIGRAPH_TOKEN_"; + +pub(crate) fn credentials_path() -> Option { + operator_dir().map(|dir| dir.join(CREDENTIALS_FILE)) +} + +/// `intel-dev` → `OMNIGRAPH_TOKEN_INTEL_DEV`. +pub(crate) fn token_env_name(server: &str) -> String { + let mut name = String::from(TOKEN_ENV_PREFIX); + for c in server.chars() { + name.push(match c { + '-' => '_', + other => other.to_ascii_uppercase(), + }); + } + name +} + +/// The keyed token chain for a named server (§D4 steps 1–2): +/// `OMNIGRAPH_TOKEN_` env → `[]` in the credentials file. +/// `Ok(None)` means "no keyed token" — callers fall through to the legacy +/// chain; a present-but-unreadable/over-permissive credentials file is a +/// loud error, never a silent skip. +pub(crate) fn resolve_keyed_token(server: &str) -> Result> { + if let Ok(token) = env::var(token_env_name(server)) { + let token = token.trim(); + if !token.is_empty() { + return Ok(Some(token.to_string())); + } + } + let Some(path) = credentials_path() else { + return Ok(None); + }; + read_credential_at(&path, server) +} + +pub(crate) fn read_credential_at(path: &Path, server: &str) -> Result> { + let text = match std::fs::read_to_string(path) { + Ok(text) => text, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(eyre!( + "could not read credentials file '{}': {err}", + path.display() + )); + } + }; + refuse_over_permissive(path)?; + let mut in_section = false; + for line in text.lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + if let Some(section) = line.strip_prefix('[').and_then(|l| l.strip_suffix(']')) { + in_section = section.trim() == server; + continue; + } + if in_section { + if let Some((key, value)) = line.split_once('=') { + if key.trim() == "token" { + let value = unquote(value.trim()); + if value.is_empty() { + return Ok(None); + } + return Ok(Some(value.to_string())); + } + } + } + } + Ok(None) +} + +/// Write (or rotate) one server's token, preserving every other section. +/// Temp file + rename (#139 finding 7), created 0600. +pub(crate) fn write_credential(server: &str, token: &str) -> Result { + let path = credentials_path() + .ok_or_else(|| eyre!("no home directory resolvable for the credentials file"))?; + rewrite_credentials_at(&path, server, Some(token))?; + Ok(path) +} + +/// Remove one server's section. Idempotent: absent file or section is fine. +pub(crate) fn remove_credential(server: &str) -> Result { + let path = credentials_path() + .ok_or_else(|| eyre!("no home directory resolvable for the credentials file"))?; + rewrite_credentials_at(&path, server, None)?; + Ok(path) +} + +pub(crate) fn rewrite_credentials_at( + path: &Path, + server: &str, + token: Option<&str>, +) -> Result<()> { + let existing = match std::fs::read_to_string(path) { + Ok(text) => { + refuse_over_permissive(path)?; + text + } + Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(err) => { + return Err(eyre!( + "could not read credentials file '{}': {err}", + path.display() + )); + } + }; + + // Drop the target section (if present), keep everything else verbatim. + let mut out = String::new(); + let mut in_target = false; + for line in existing.lines() { + let trimmed = line.trim(); + if let Some(section) = trimmed.strip_prefix('[').and_then(|l| l.strip_suffix(']')) { + in_target = section.trim() == server; + if in_target { + continue; + } + } + if !in_target { + out.push_str(line); + out.push('\n'); + } + } + if let Some(token) = token { + if !out.is_empty() && !out.ends_with("\n\n") { + out.push('\n'); + } + out.push_str(&format!("[{server}]\ntoken = {token}\n")); + } + + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let tmp = path.with_extension(format!("tmp.{}", std::process::id())); + write_owner_only(&tmp, &out)?; + std::fs::rename(&tmp, path).map_err(|err| { + let _ = std::fs::remove_file(&tmp); + eyre!( + "could not move credentials file into place '{}': {err}", + path.display() + ) + })?; + Ok(()) +} + +#[cfg(unix)] +fn write_owner_only(path: &Path, content: &str) -> Result<()> { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + let mut file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(path)?; + file.write_all(content.as_bytes())?; + Ok(()) +} + +#[cfg(not(unix))] +fn write_owner_only(path: &Path, content: &str) -> Result<()> { + std::fs::write(path, content)?; + Ok(()) +} + +/// Secrets are operator-private: refuse a credentials file other accounts +/// can read (the chain errs loudly rather than using a leaked secret). +#[cfg(unix)] +fn refuse_over_permissive(path: &Path) -> Result<()> { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(path)?.permissions().mode(); + if mode & 0o077 != 0 { + return Err(eyre!( + "credentials file '{}' is group/world-accessible (mode {:o}); run `chmod 600 {}`", + path.display(), + mode & 0o777, + path.display() + )); + } + Ok(()) +} + +#[cfg(not(unix))] +fn refuse_over_permissive(_path: &Path) -> Result<()> { + Ok(()) +} + +fn unquote(value: &str) -> &str { + if value.len() >= 2 + && ((value.starts_with('"') && value.ends_with('"')) + || (value.starts_with('\'') && value.ends_with('\''))) + { + &value[1..value.len() - 1] + } else { + value + } +} + /// Expand a leading `~` / `~/` to the home directory (PR #139 finding 9: /// a literal `./~/…` path silently created a directory named `~`). pub(crate) fn expand_tilde(raw: &str) -> PathBuf { @@ -186,10 +425,12 @@ mod tests { let config = load_operator_config_at(&path).unwrap(); assert_eq!(config.actor(), Some("act-a")); let warnings = config.unknown_key_warnings(); - assert_eq!(warnings.len(), 3, "{warnings:?}"); - assert!(warnings.iter().any(|w| w.contains("`servers`"))); + // `servers` became a known key in PR 2; `aliases` stays unknown + // until PR 3. + assert_eq!(warnings.len(), 2, "{warnings:?}"); assert!(warnings.iter().any(|w| w.contains("`aliases`"))); assert!(warnings.iter().any(|w| w.contains("`operator.color`"))); + assert_eq!(config.servers["prod"].url, "https://example.com"); } #[test] @@ -201,6 +442,85 @@ mod tests { assert!(err.to_string().contains("could not parse operator config")); } + #[test] + fn find_server_for_url_longest_prefix_no_substring_traps() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.yaml"); + fs::write( + &path, + "servers:\n dev:\n url: http://h:8080\n dev-spike:\n url: http://h:8080/graphs/spike\n", + ) + .unwrap(); + let config = load_operator_config_at(&path).unwrap(); + assert_eq!(config.find_server_for_url("http://h:8080"), Some("dev")); + assert_eq!( + config.find_server_for_url("http://h:8080/graphs/other"), + Some("dev") + ); + // longest prefix wins + assert_eq!( + config.find_server_for_url("http://h:8080/graphs/spike/queries/q"), + Some("dev-spike") + ); + // no substring trap: a different port/host must not match + assert_eq!(config.find_server_for_url("http://h:8080-evil/x"), None); + assert_eq!(config.find_server_for_url("http://other:9999"), None); + } + + #[test] + fn token_env_name_uppercases_and_underscores() { + assert_eq!(token_env_name("intel-dev"), "OMNIGRAPH_TOKEN_INTEL_DEV"); + assert_eq!(token_env_name("prod"), "OMNIGRAPH_TOKEN_PROD"); + } + + #[test] + fn credentials_roundtrip_rotate_remove_preserving_other_sections() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials"); + + rewrite_credentials_at(&path, "prod", Some("tok-1")).unwrap(); + rewrite_credentials_at(&path, "dev", Some("tok-dev")).unwrap(); + assert_eq!( + read_credential_at(&path, "prod").unwrap().as_deref(), + Some("tok-1") + ); + + // rotate prod; dev preserved + rewrite_credentials_at(&path, "prod", Some("tok-2")).unwrap(); + assert_eq!( + read_credential_at(&path, "prod").unwrap().as_deref(), + Some("tok-2") + ); + assert_eq!( + read_credential_at(&path, "dev").unwrap().as_deref(), + Some("tok-dev") + ); + + // remove prod; dev preserved; removal is idempotent + rewrite_credentials_at(&path, "prod", None).unwrap(); + rewrite_credentials_at(&path, "prod", None).unwrap(); + assert_eq!(read_credential_at(&path, "prod").unwrap(), None); + assert_eq!( + read_credential_at(&path, "dev").unwrap().as_deref(), + Some("tok-dev") + ); + } + + #[cfg(unix)] + #[test] + fn credentials_written_0600_and_over_permissive_refused() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials"); + rewrite_credentials_at(&path, "prod", Some("tok")).unwrap(); + let mode = fs::metadata(&path).unwrap().permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "written {:o}", mode & 0o777); + + fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); + let err = read_credential_at(&path, "prod").unwrap_err(); + assert!(err.to_string().contains("chmod 600"), "{err}"); + } + #[test] fn expand_tilde_resolves_home_prefix() { let home = env::home_dir().unwrap(); diff --git a/crates/omnigraph-cli/src/output.rs b/crates/omnigraph-cli/src/output.rs index f77e50f..04df60a 100644 --- a/crates/omnigraph-cli/src/output.rs +++ b/crates/omnigraph-cli/src/output.rs @@ -828,3 +828,48 @@ pub(crate) struct QueriesListItem { pub(crate) struct QueriesListOutput { pub(crate) queries: Vec, } + +pub(crate) fn finish_login( + server: &str, + credentials_path: &std::path::Path, + declared: bool, + json: bool, +) -> Result<()> { + if json { + print_json(&serde_json::json!({ + "server": server, + "credentials_path": credentials_path.display().to_string(), + "declared": declared, + }))?; + } else { + println!( + "stored credential for '{server}' in {}", + credentials_path.display() + ); + } + if !declared { + eprintln!( + "note: '{server}' is not declared under servers: in the operator config; the token applies once you add `servers:\n {server}:\n url: ` to ~/.omnigraph/config.yaml" + ); + } + Ok(()) +} + +pub(crate) fn finish_logout( + server: &str, + credentials_path: &std::path::Path, + json: bool, +) -> Result<()> { + if json { + print_json(&serde_json::json!({ + "server": server, + "credentials_path": credentials_path.display().to_string(), + }))?; + } else { + println!( + "removed credential for '{server}' from {}", + credentials_path.display() + ); + } + Ok(()) +} diff --git a/crates/omnigraph-cli/tests/support/mod.rs b/crates/omnigraph-cli/tests/support/mod.rs index 41e46c7..b11e94d 100644 --- a/crates/omnigraph-cli/tests/support/mod.rs +++ b/crates/omnigraph-cli/tests/support/mod.rs @@ -259,6 +259,15 @@ pub fn spawn_server_with_cluster_env(cluster_dir: &Path, envs: &[(&str, &str)]) spawn_server_process(command) } +pub fn spawn_server_with_env(graph: &Path, envs: &[(&str, &str)]) -> TestServer { + let mut command = server_process(); + command.arg(graph); + for (name, value) in envs { + command.env(name, value); + } + spawn_server_process(command) +} + pub fn spawn_server_with_config_env(config: &Path, envs: &[(&str, &str)]) -> TestServer { let mut command = server_process(); command.arg("--config").arg(config); diff --git a/crates/omnigraph-cli/tests/system_local.rs b/crates/omnigraph-cli/tests/system_local.rs index 46f6fcf..5eb739f 100644 --- a/crates/omnigraph-cli/tests/system_local.rs +++ b/crates/omnigraph-cli/tests/system_local.rs @@ -2309,3 +2309,118 @@ fn cluster_server_boot_ignores_local_config_in_cwd() { let response = reqwest::blocking::get(format!("{}/healthz", server.base_url)).unwrap(); assert!(response.status().is_success()); } + +/// RFC-007 PR 2: keyed credentials end to end — `login` stores a 0600 +/// credential, the URL-matched server's token chain authenticates remote +/// reads (env > file), a non-matching URL never sees the token (§D5 rule +/// 3), and `logout` revokes. +#[test] +fn local_cli_keyed_credentials_authenticate_url_matched_server() { + let graph = SystemGraph::loaded(); + let server = spawn_server_with_env( + graph.path(), + &[("OMNIGRAPH_SERVER_BEARER_TOKEN", "secret-tok")], + ); + let operator_home = tempfile::tempdir().unwrap(); + let write_server_url = |url: &str| { + fs::write( + operator_home.path().join("config.yaml"), + format!("servers:\n test-srv:\n url: {url}\n"), + ) + .unwrap(); + }; + write_server_url(&server.base_url); + + let remote_read = |envs: &[(&str, &str)]| { + let mut command = cli(); + command.env("OMNIGRAPH_HOME", operator_home.path()); + for (name, value) in envs { + command.env(name, value); + } + command + .arg("read") + .arg(&server.base_url) + .arg("--query") + .arg(fixture("test.gq")) + .arg("--name") + .arg("get_person") + .arg("--params") + .arg(r#"{"name":"Alice"}"#) + .arg("--json") + .output() + .unwrap() + }; + + // No credential anywhere: the server refuses. + let output = remote_read(&[]); + assert!(!output.status.success(), "{output:?}"); + + // login with a WRONG token (via stdin, the documented pipe flow). + let output = cli() + .env("OMNIGRAPH_HOME", operator_home.path()) + .arg("login") + .arg("test-srv") + .write_stdin("wrong-tok\n") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + let output = remote_read(&[]); + assert!(!output.status.success(), "wrong token must not authenticate"); + + // Re-login rotates to the right token (via --token); 0600 on disk. + let output = cli() + .env("OMNIGRAPH_HOME", operator_home.path()) + .arg("login") + .arg("test-srv") + .arg("--token") + .arg("secret-tok") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + let credentials = operator_home.path().join("credentials"); + let text = fs::read_to_string(&credentials).unwrap(); + assert!(text.contains("[test-srv]"), "{text}"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = fs::metadata(&credentials).unwrap().permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "{:o}", mode & 0o777); + } + let output = remote_read(&[]); + assert!( + output.status.success(), + "keyed credential must authenticate the URL-matched server: {output:?}" + ); + let payload: serde_json::Value = + serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(payload["rows"][0]["p.name"], "Alice"); + + // OMNIGRAPH_TOKEN_ env outranks the credentials file. + let output = remote_read(&[("OMNIGRAPH_TOKEN_TEST_SRV", "env-wrong")]); + assert!( + !output.status.success(), + "keyed env token must outrank the credentials file" + ); + + // §D5 rule 3: a URL matching no operator server never sees the token. + write_server_url("http://127.0.0.1:1"); + let output = remote_read(&[]); + assert!( + !output.status.success(), + "token keyed to another url must not be sent here" + ); + write_server_url(&server.base_url); + + // logout revokes; idempotent. + for _ in 0..2 { + let output = cli() + .env("OMNIGRAPH_HOME", operator_home.path()) + .arg("logout") + .arg("test-srv") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + } + let output = remote_read(&[]); + assert!(!output.status.success(), "logout must revoke access"); +} diff --git a/docs/dev/rfc-007-operator-config.md b/docs/dev/rfc-007-operator-config.md index 52a446e..5abf4e1 100644 --- a/docs/dev/rfc-007-operator-config.md +++ b/docs/dev/rfc-007-operator-config.md @@ -241,13 +241,13 @@ future checkout-supplied surface: Three PRs, each independently useful, each landable without the next: -1. **PR 1 — the operator file + identity.** Loader for +1. **PR 1 — the operator file + identity** *(landed: #196)*. Loader for `~/.omnigraph/config.yaml` (+ `OMNIGRAPH_HOME`, `~`-expansion, warn-only unknown keys), `operator.actor` joining the `--as` cascade, `defaults.output` joining the format cascade, `OMNIGRAPH_CONFIG` env for the CLI's `--config`. Docs: `cli-reference.md` gains the two-surface table. -2. **PR 2 — keyed credentials.** `servers:` in the operator layer, the +2. **PR 2 — keyed credentials** *(landed)*. `servers:` in the operator layer, the §D4 chain (env + credentials file), the §D5 trust rules, and `omnigraph login ` (atomic write, `0600`). Legacy mechanisms untouched and tested-as-untouched. diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 1dbc1ff..c41a15c 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -48,6 +48,9 @@ listed there. operator: actor: act-andrew # default identity for every --as cascade: # --as > legacy cli.actor > operator.actor > none +servers: # operator-owned endpoints; names key the credentials + prod: + url: https://graph.example.com # no tokens in this file, ever defaults: output: table # read format default, below --json/--format/alias/legacy ``` @@ -56,6 +59,26 @@ Absent file = empty layer. Unknown keys warn and load (a file written for a newer CLI works on an older one). `$OMNIGRAPH_CONFIG=` stands in for `--config` (the flag wins) in both the CLI and the server. +#### Credentials keyed by server name + +`omnigraph login ` stores a bearer token in +`~/.omnigraph/credentials` (created `0600`; group/world-readable files are +refused). Token from `--token`, or — preferred, keeps it out of shell +history — one line on stdin: `echo $TOKEN | omnigraph login prod`. +`omnigraph logout ` removes it (idempotent). + +A remote command whose URL prefix-matches an operator server's `url` (the +`gh` host model — no flags needed) resolves its token through: + +| Order | Source | +|---|---| +| 1 | `OMNIGRAPH_TOKEN_` env (`prod` → `OMNIGRAPH_TOKEN_PROD`) | +| 2 | `[]` section in `~/.omnigraph/credentials` | +| 3 | the legacy chain unchanged (`bearer_token_env` → `OMNIGRAPH_BEARER_TOKEN` → `auth.env_file`) | + +A token is only ever sent to the server it is keyed to: URLs matching no +operator server use the legacy chain alone. + ## `omnigraph.yaml` schema (legacy combined file) ```yaml