mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-07-12 03:12:11 +02:00
policy: server 3-state default-deny matrix (MR-723) (#105)
Closes the "tokens but no policy" trap. Pre-MR-723, an operator who configured bearer tokens and forgot to set policy.file got a server that required auth and then permitted every action — the illusion of protection. After MR-723, that configuration is default-deny: only `read` actions succeed; every other action returns HTTP 403. Three startup states, classified deterministically: - **Open** — no tokens, no policy. Requires explicit `--unauthenticated` flag or `OMNIGRAPH_UNAUTHENTICATED=1`; otherwise `serve()` refuses to start. Forces the operator to opt in to "fully open dev mode" so it can't happen accidentally. - **DefaultDeny** — tokens configured, no policy. `authorize_request` rejects every action except `Read` with 403. The warn-log on startup names the misconfiguration explicitly. - **PolicyEnabled** — policy file configured. Cedar evaluates every request, unchanged from pre-MR-723. What landed: - `ServerConfig.allow_unauthenticated: bool` + `--unauthenticated` flag on the `omnigraph-server` bin + `OMNIGRAPH_UNAUTHENTICATED` env var (`load_server_settings` honors both). - New `classify_server_runtime_state(has_tokens, has_policy, allow_unauthenticated) -> Result<ServerRuntimeState>` pure function. `serve()` calls it before opening the engine and bails with a clear error when the operator hits the no-tokens-no-policy-no-flag cell. - `authorize_request` state-2 branch: when `policy_engine()` is None but the bearer-auth middleware delivered an authenticated actor, any action other than `Read` returns 403 with a message that names the misconfiguration. - `AppState::with_policy_engine(self, engine)` builder method so integration tests that need a custom workload (`new_with_workload`) can still install a permit-all policy without a new constructor. - `app_for_loaded_repo_with_auth(token)` and `app_for_loaded_repo_with_auth_tokens(tokens)` test helpers now install a permit-all policy alongside tokens — they previously represented the "tokens but no policy" state that MR-723 makes default-deny, and tests that don't care about policy were inadvertently coupled to the loophole. Tests: - `classify_*` unit tests (3) — every cell of the matrix. - `default_deny_mode_allows_read_for_authenticated_actor` — GET /snapshot succeeds with bearer token + no policy. - `default_deny_mode_rejects_change_with_forbidden` — POST /change rejected with 403 + "default-deny" message. - `default_deny_mode_rejects_schema_apply_with_forbidden` — POST /schema/apply rejected with 403 + "default-deny" message. - New `app_for_repo_with_auth_tokens_only(schema, tokens)` helper builds the State-2 fixture without policy. The pre-MR-723 helpers `app_for_loaded_repo_with_auth*` shift semantics to "tokens + permit-all" so existing tests retain their original intent. docs/user/policy.md: new "Server runtime states (MR-723)" section documents the matrix and the explicit `--unauthenticated` opt-in. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
a275306a15
commit
f3f2a051ba
4 changed files with 407 additions and 23 deletions
|
|
@ -50,7 +50,7 @@ use subtle::ConstantTimeEq;
|
|||
use tokio::net::TcpListener;
|
||||
use tokio::sync::mpsc;
|
||||
use tower_http::trace::TraceLayer;
|
||||
use tracing::{error, info};
|
||||
use tracing::{error, info, warn};
|
||||
use tracing_subscriber::EnvFilter;
|
||||
use utoipa::OpenApi;
|
||||
use utoipa::openapi::security::{Http, HttpAuthScheme, SecurityScheme};
|
||||
|
|
@ -114,6 +114,15 @@ pub struct ServerConfig {
|
|||
pub uri: String,
|
||||
pub bind: String,
|
||||
pub policy_file: Option<PathBuf>,
|
||||
/// Operator opt-in for fully-unauthenticated dev mode (MR-723).
|
||||
/// When neither bearer tokens nor a policy file are configured,
|
||||
/// `serve()` refuses to start unless this is true (set via
|
||||
/// `--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1`). The
|
||||
/// motivation is that "no tokens + no policy" looks like protection
|
||||
/// (no Cedar errors at boot) but is actually fully open — operators
|
||||
/// who set up auth and forgot the policy file would otherwise ship
|
||||
/// the illusion of protection.
|
||||
pub allow_unauthenticated: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
|
|
@ -246,6 +255,17 @@ impl AppState {
|
|||
}
|
||||
}
|
||||
|
||||
/// Install a `PolicyEngine` post-construction (MR-723). Used by
|
||||
/// integration tests that need to thread custom workload limits
|
||||
/// alongside a permit-all policy — the existing `new_with_*` and
|
||||
/// `new_with_workload` constructors don't compose. Production
|
||||
/// callers should use `open_with_bearer_tokens_and_policy` which
|
||||
/// installs the policy on both the HTTP state and the engine.
|
||||
pub fn with_policy_engine(mut self, engine: PolicyEngine) -> Self {
|
||||
self.policy_engine = Some(Arc::new(engine));
|
||||
self
|
||||
}
|
||||
|
||||
pub async fn open(uri: impl Into<String>) -> Result<Self> {
|
||||
Self::open_with_bearer_token(uri, None).await
|
||||
}
|
||||
|
|
@ -535,20 +555,77 @@ pub fn load_server_settings(
|
|||
cli_uri: Option<String>,
|
||||
cli_target: Option<String>,
|
||||
cli_bind: Option<String>,
|
||||
cli_allow_unauthenticated: bool,
|
||||
) -> Result<ServerConfig> {
|
||||
let config = load_config(config_path)?;
|
||||
let uri =
|
||||
config.resolve_target_uri(cli_uri, cli_target.as_deref(), config.server_graph_name())?;
|
||||
let bind = cli_bind.unwrap_or_else(|| config.server_bind().to_string());
|
||||
let policy_file = config.resolve_policy_file();
|
||||
// Either `--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1` flips
|
||||
// this. Treat any non-empty, non-"0"/"false" string as truthy —
|
||||
// standard 12-factor "any value is true" reading of the env var.
|
||||
let env_unauth = std::env::var("OMNIGRAPH_UNAUTHENTICATED")
|
||||
.ok()
|
||||
.map(|v| {
|
||||
let trimmed = v.trim();
|
||||
!trimmed.is_empty() && trimmed != "0" && !trimmed.eq_ignore_ascii_case("false")
|
||||
})
|
||||
.unwrap_or(false);
|
||||
let allow_unauthenticated = cli_allow_unauthenticated || env_unauth;
|
||||
|
||||
Ok(ServerConfig {
|
||||
uri,
|
||||
bind,
|
||||
policy_file,
|
||||
allow_unauthenticated,
|
||||
})
|
||||
}
|
||||
|
||||
/// MR-723 server runtime state, classified from the three-state matrix
|
||||
/// of (bearer tokens configured) × (policy file configured) at startup.
|
||||
///
|
||||
/// * **Open** — neither tokens nor policy; requires explicit
|
||||
/// `allow_unauthenticated`. Effectively a "trust the network" dev
|
||||
/// mode. `serve()` refuses to start in this shape without the flag,
|
||||
/// so the only way to reach this state at runtime is via deliberate
|
||||
/// operator opt-in.
|
||||
/// * **DefaultDeny** — tokens configured but no policy file. The
|
||||
/// server requires a valid bearer token; once authenticated, every
|
||||
/// action except `Read` is denied with 403. Closes the "tokens but
|
||||
/// forgot the policy file" trap.
|
||||
/// * **PolicyEnabled** — policy file configured. Cedar evaluates every
|
||||
/// authenticated request. Tokens may also be configured (typical) or
|
||||
/// not (unusual but valid — every request fails 401 without a
|
||||
/// bearer, which is effectively "locked").
|
||||
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
|
||||
pub enum ServerRuntimeState {
|
||||
Open,
|
||||
DefaultDeny,
|
||||
PolicyEnabled,
|
||||
}
|
||||
|
||||
/// Compute the [`ServerRuntimeState`] from the configured inputs.
|
||||
/// Pulled out as a pure function so the 3-state matrix is unit-testable
|
||||
/// without standing up the full server.
|
||||
pub fn classify_server_runtime_state(
|
||||
has_tokens: bool,
|
||||
has_policy: bool,
|
||||
allow_unauthenticated: bool,
|
||||
) -> Result<ServerRuntimeState> {
|
||||
match (has_tokens, has_policy, allow_unauthenticated) {
|
||||
(false, false, false) => bail!(
|
||||
"server has no bearer tokens and no policy file configured. This is a fully \
|
||||
open server — pass `--unauthenticated` (or set OMNIGRAPH_UNAUTHENTICATED=1) \
|
||||
if you actually want that, otherwise configure bearer tokens (see \
|
||||
docs/user/server.md) and/or `policy.file` in omnigraph.yaml."
|
||||
),
|
||||
(false, false, true) => Ok(ServerRuntimeState::Open),
|
||||
(true, false, _) => Ok(ServerRuntimeState::DefaultDeny),
|
||||
(_, true, _) => Ok(ServerRuntimeState::PolicyEnabled),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn build_app(state: AppState) -> Router {
|
||||
let protected = Router::new()
|
||||
.route("/snapshot", get(server_snapshot))
|
||||
|
|
@ -586,9 +663,28 @@ pub fn build_app(state: AppState) -> Router {
|
|||
pub async fn serve(config: ServerConfig) -> Result<()> {
|
||||
let token_source = resolve_token_source().await?;
|
||||
info!(source = token_source.name(), "loaded bearer token source");
|
||||
let tokens = token_source.load().await?;
|
||||
let runtime_state = classify_server_runtime_state(
|
||||
!tokens.is_empty(),
|
||||
config.policy_file.is_some(),
|
||||
config.allow_unauthenticated,
|
||||
)?;
|
||||
match runtime_state {
|
||||
ServerRuntimeState::Open => warn!(
|
||||
"running with --unauthenticated: no bearer tokens, no policy file, all \
|
||||
requests permitted. This is for local dev only — do not expose to a \
|
||||
network you don't fully trust."
|
||||
),
|
||||
ServerRuntimeState::DefaultDeny => warn!(
|
||||
"bearer tokens are configured but no policy file is set — running in \
|
||||
default-deny mode (only `read` actions are permitted for authenticated \
|
||||
actors). Configure `policy.file` in omnigraph.yaml to enable Cedar rules."
|
||||
),
|
||||
ServerRuntimeState::PolicyEnabled => {}
|
||||
}
|
||||
let state = AppState::open_with_bearer_tokens_and_policy(
|
||||
config.uri.clone(),
|
||||
token_source.load().await?,
|
||||
tokens,
|
||||
config.policy_file.as_ref(),
|
||||
)
|
||||
.await?;
|
||||
|
|
@ -708,6 +804,27 @@ fn authorize_request(
|
|||
mut request: PolicyRequest,
|
||||
) -> std::result::Result<(), ApiError> {
|
||||
let Some(engine) = state.policy_engine() else {
|
||||
// MR-723 default-deny path. We're here when no PolicyEngine is
|
||||
// installed. Two startup-validated shapes can reach this:
|
||||
//
|
||||
// * **Open mode** (`--unauthenticated`): no tokens, no policy.
|
||||
// `require_bearer_auth` short-circuits before this is called,
|
||||
// but defense in depth — if a future change makes the
|
||||
// middleware call here for an unauthenticated request, we
|
||||
// want every action to remain Ok rather than 403. The
|
||||
// operator opted in.
|
||||
// * **DefaultDeny mode**: tokens configured but no policy. The
|
||||
// request went through bearer auth, so `actor` is Some and
|
||||
// identifies a known actor. Only `Read` is permitted; every
|
||||
// other action returns 403. This closes the "configured auth
|
||||
// but forgot the policy file" trap from MR-723.
|
||||
if actor.is_some() && request.action != PolicyAction::Read {
|
||||
return Err(ApiError::forbidden(
|
||||
"server runs in default-deny mode (bearer tokens configured but no \
|
||||
policy file). Only `read` actions are permitted; configure \
|
||||
`policy.file` in omnigraph.yaml to enable other actions.",
|
||||
));
|
||||
}
|
||||
return Ok(());
|
||||
};
|
||||
let Some(actor) = actor else {
|
||||
|
|
@ -1641,8 +1758,8 @@ fn server_bearer_tokens_from_env() -> Result<Vec<(String, String)>> {
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{
|
||||
hash_bearer_token, load_server_settings, normalize_bearer_token, parse_bearer_tokens_json,
|
||||
server_bearer_tokens_from_env,
|
||||
ServerRuntimeState, classify_server_runtime_state, hash_bearer_token, load_server_settings,
|
||||
normalize_bearer_token, parse_bearer_tokens_json, server_bearer_tokens_from_env,
|
||||
};
|
||||
use std::env;
|
||||
use std::fs;
|
||||
|
|
@ -1695,7 +1812,7 @@ server:
|
|||
)
|
||||
.unwrap();
|
||||
|
||||
let settings = load_server_settings(Some(&config), None, None, None).unwrap();
|
||||
let settings = load_server_settings(Some(&config), None, None, None, false).unwrap();
|
||||
assert_eq!(settings.uri, "/tmp/demo.omni");
|
||||
assert_eq!(settings.bind, "0.0.0.0:9090");
|
||||
}
|
||||
|
|
@ -1722,6 +1839,7 @@ server:
|
|||
Some("/tmp/override.omni".to_string()),
|
||||
None,
|
||||
Some("0.0.0.0:9999".to_string()),
|
||||
false,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(settings.uri, "/tmp/override.omni");
|
||||
|
|
@ -1748,16 +1866,69 @@ server:
|
|||
.unwrap();
|
||||
|
||||
let settings =
|
||||
load_server_settings(Some(&config), None, Some("dev".to_string()), None).unwrap();
|
||||
load_server_settings(Some(&config), None, Some("dev".to_string()), None, false)
|
||||
.unwrap();
|
||||
assert_eq!(settings.uri, "http://127.0.0.1:8080");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_settings_require_uri_from_cli_or_config() {
|
||||
let error = load_server_settings(None, None, None, None).unwrap_err();
|
||||
let error = load_server_settings(None, None, None, None, false).unwrap_err();
|
||||
assert!(error.to_string().contains("URI must be provided"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_open_requires_explicit_unauthenticated_flag() {
|
||||
// State 1: no tokens, no policy, no flag → refuse to start.
|
||||
let error = classify_server_runtime_state(false, false, false).unwrap_err();
|
||||
let msg = error.to_string();
|
||||
assert!(
|
||||
msg.contains("--unauthenticated"),
|
||||
"expected refusal message mentioning --unauthenticated, got: {msg}"
|
||||
);
|
||||
|
||||
// Same matrix cell but with the flag set → Open mode permitted.
|
||||
assert_eq!(
|
||||
classify_server_runtime_state(false, false, true).unwrap(),
|
||||
ServerRuntimeState::Open
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_tokens_without_policy_is_default_deny() {
|
||||
// State 2: tokens configured, no policy → DefaultDeny regardless
|
||||
// of the flag (the flag opts into the fully-open dev mode; it
|
||||
// doesn't downgrade default-deny back to open).
|
||||
assert_eq!(
|
||||
classify_server_runtime_state(true, false, false).unwrap(),
|
||||
ServerRuntimeState::DefaultDeny
|
||||
);
|
||||
assert_eq!(
|
||||
classify_server_runtime_state(true, false, true).unwrap(),
|
||||
ServerRuntimeState::DefaultDeny
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_policy_enabled_always_wins() {
|
||||
// State 3: any setup with a policy file → PolicyEnabled. The
|
||||
// flag doesn't matter and tokens-or-not doesn't matter (no
|
||||
// tokens + policy is unusual but valid — every request fails
|
||||
// 401 without a bearer, which is effectively "locked").
|
||||
assert_eq!(
|
||||
classify_server_runtime_state(true, true, false).unwrap(),
|
||||
ServerRuntimeState::PolicyEnabled
|
||||
);
|
||||
assert_eq!(
|
||||
classify_server_runtime_state(false, true, false).unwrap(),
|
||||
ServerRuntimeState::PolicyEnabled
|
||||
);
|
||||
assert_eq!(
|
||||
classify_server_runtime_state(true, true, true).unwrap(),
|
||||
ServerRuntimeState::PolicyEnabled
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_bearer_token_trims_and_filters_blank_values() {
|
||||
assert_eq!(normalize_bearer_token(None), None);
|
||||
|
|
|
|||
|
|
@ -16,6 +16,12 @@ struct Cli {
|
|||
config: Option<PathBuf>,
|
||||
#[arg(long)]
|
||||
bind: Option<String>,
|
||||
/// Run without bearer tokens and without a policy file (MR-723).
|
||||
/// Required when neither is configured — otherwise the server
|
||||
/// refuses to start to prevent shipping the illusion of protection.
|
||||
/// Equivalent to setting `OMNIGRAPH_UNAUTHENTICATED=1`.
|
||||
#[arg(long)]
|
||||
unauthenticated: bool,
|
||||
}
|
||||
|
||||
#[tokio::main]
|
||||
|
|
@ -24,7 +30,12 @@ async fn main() -> Result<()> {
|
|||
init_tracing();
|
||||
|
||||
let cli = Cli::parse();
|
||||
let settings: ServerConfig =
|
||||
load_server_settings(cli.config.as_ref(), cli.uri, cli.target, cli.bind)?;
|
||||
let settings: ServerConfig = load_server_settings(
|
||||
cli.config.as_ref(),
|
||||
cli.uri,
|
||||
cli.target,
|
||||
cli.bind,
|
||||
cli.unauthenticated,
|
||||
)?;
|
||||
serve(settings).await
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue