chore: address review cleanup comments

This commit is contained in:
Ragnor Comerford 2026-05-28 16:09:44 +02:00
parent eab99e6f48
commit 4580ec011a
No known key found for this signature in database
4 changed files with 37 additions and 41 deletions

View file

@ -210,23 +210,17 @@ impl OmnigraphConfig {
}
pub fn resolve_auth_env_file(&self) -> Option<PathBuf> {
let path = self.auth.env_file.as_deref()?;
let path = Path::new(path);
Some(if path.is_absolute() {
path.to_path_buf()
} else {
self.base_dir.join(path)
})
self.auth
.env_file
.as_deref()
.map(|path| self.resolve_config_path(path))
}
pub fn resolve_policy_file(&self) -> Option<PathBuf> {
let path = self.policy.file.as_deref()?;
let path = Path::new(path);
Some(if path.is_absolute() {
path.to_path_buf()
} else {
self.base_dir.join(path)
})
self.policy
.file
.as_deref()
.map(|path| self.resolve_config_path(path))
}
/// Resolve the per-graph policy file path for the named target,
@ -234,25 +228,21 @@ impl OmnigraphConfig {
/// target is unknown or no per-graph `policy.file` is set.
pub fn resolve_target_policy_file(&self, target_name: &str) -> Option<PathBuf> {
let target = self.graphs.get(target_name)?;
let path = target.policy.file.as_deref()?;
let path = Path::new(path);
Some(if path.is_absolute() {
path.to_path_buf()
} else {
self.base_dir.join(path)
})
target
.policy
.file
.as_deref()
.map(|path| self.resolve_config_path(path))
}
/// Resolve the server-level policy file path (used by management
/// endpoints). Returns `None` if `server.policy.file` is not set.
pub fn resolve_server_policy_file(&self) -> Option<PathBuf> {
let path = self.server.policy.file.as_deref()?;
let path = Path::new(path);
Some(if path.is_absolute() {
path.to_path_buf()
} else {
self.base_dir.join(path)
})
self.server
.policy
.file
.as_deref()
.map(|path| self.resolve_config_path(path))
}
/// Resolve a raw config-supplied URI (which may be relative) to its
@ -328,6 +318,15 @@ impl OmnigraphConfig {
self.base_dir.join(path).to_string_lossy().to_string()
}
}
fn resolve_config_path(&self, value: &str) -> PathBuf {
let path = Path::new(value);
if path.is_absolute() {
path.to_path_buf()
} else {
self.base_dir.join(path)
}
}
}
pub fn default_config_path() -> PathBuf {

View file

@ -5270,12 +5270,11 @@ rules:
);
}
/// End-to-end: load an `omnigraph.yaml` with two graphs and serve
/// them. Both graphs must be queryable via cluster routes.
///
/// Uses `_` placeholders for tempdirs so they live until end-of-test.
/// Loads an `omnigraph.yaml` with two graphs and verifies multi-mode
/// inference plus graph entry resolution. Cluster-route dispatch is
/// covered by the route tests above.
#[tokio::test(flavor = "multi_thread")]
async fn server_settings_drive_multi_graph_startup_end_to_end() {
async fn server_settings_load_multi_graph_config_entries() {
let cfg_dir = tempfile::tempdir().unwrap();
// Real graph storage dirs (the URIs in the config must point to
// a graph init-able location).
@ -5310,9 +5309,6 @@ graphs:
load_server_settings(Some(&config_path), None, None, None, true).unwrap();
assert!(matches!(settings.mode, ServerConfigMode::Multi { .. }));
// We don't actually call `serve()` (would bind a socket); we
// just confirm the settings are well-formed and the inferred
// mode lists both graphs.
match settings.mode {
ServerConfigMode::Multi { graphs, .. } => {
assert_eq!(graphs.len(), 2);

View file

@ -79,7 +79,7 @@ cli:
actor: act-andrew # default actor for CLI direct-engine writes
```
Each per-graph rule must use exactly one of `branch_scope` or `target_branch_scope`. Server-scoped rules (`graph_list`) take neither — they have no branch context.
Each per-graph rule may use at most one of `branch_scope` or `target_branch_scope`. Server-scoped rules (`graph_list`) take neither — they have no branch context.
`cli.actor` is the default actor identity for CLI direct-engine writes
when `policy.file` is configured. Override per-invocation with `--as
@ -121,7 +121,7 @@ reaches `authorize_request()` without a matching policy permit.
|---|---|---|---|
| **Open** | no | no | Every request is permitted. Refuses to start unless `--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1` is set — the operator must explicitly opt in. |
| **DefaultDeny** | yes | no | Every authenticated request for an action other than `read` is rejected with HTTP 403. Closes the "tokens but forgot the policy file" trap — an operator who sets up auth and forgot to point at a policy file used to ship the illusion of protection. |
| **PolicyEnabled** | yes | yes | Every request is evaluated by Cedar against the configured policy. |
| **PolicyEnabled** | yes | yes | Authenticated requests that reach a configured policy engine are evaluated by Cedar. Server-scoped actions still require `server.policy.file`. |
The classifier is `classify_server_runtime_state` in
`crates/omnigraph-server/src/lib.rs`; it returns `Err` for the "no

View file

@ -117,9 +117,10 @@ endpoints (`/snapshot`, `/read`, `/export`, `/branches` GET, `/commits`,
1. `OMNIGRAPH_SERVER_BEARER_TOKENS_AWS_SECRET` — AWS Secrets Manager (build with `--features aws`)
2. `OMNIGRAPH_SERVER_BEARER_TOKENS_FILE` or `OMNIGRAPH_SERVER_BEARER_TOKENS_JSON` — JSON `{actor_id: token, …}`
3. `OMNIGRAPH_SERVER_BEARER_TOKEN` — single legacy token, actor `default`
- If no tokens and no policy are configured, startup refuses unless
`--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1` explicitly opts into
open local-dev mode. In that mode `/openapi.json` strips the security scheme.
- If no tokens are configured, startup refuses unless `--unauthenticated` or
`OMNIGRAPH_UNAUTHENTICATED=1` explicitly opts into open local-dev mode. A
policy file without tokens is also rejected at startup. In open mode
`/openapi.json` strips the security scheme.
See [deployment.md](deployment.md) for token-source operational details.