From 7129c3cd3f4debe03504dceb217a4eb388f9637f Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Tue, 26 May 2026 15:12:36 -0700 Subject: [PATCH 1/7] feat(server): per-handler gRPC auth annotations Move scope, role, and auth-mode metadata to the handler definition site via #[rpc_authz] + #[rpc_auth] proc macros. The previously hand-maintained SCOPED_METHODS, ADMIN_METHODS, UNAUTHENTICATED_METHODS, and ALLOWED_SANDBOX_METHODS tables are now generated from per-method annotations on the tonic service impls, with canonical gRPC paths derived from the service name and method name. Adds a new openshell-server-macros proc-macro crate, an aggregator in auth/method_authz.rs, and an exhaustiveness test that decodes the protobuf FileDescriptorSet (now emitted by openshell-core/build.rs) and verifies every RPC has an annotation. Signed-off-by: Mrunal Patel --- Cargo.lock | 10 + crates/openshell-core/build.rs | 11 + crates/openshell-core/src/lib.rs | 7 + crates/openshell-server-macros/Cargo.toml | 18 + crates/openshell-server-macros/src/lib.rs | 343 ++++++++++++++++++ crates/openshell-server/Cargo.toml | 1 + crates/openshell-server/src/auth/authz.rs | 129 +------ .../openshell-server/src/auth/method_authz.rs | 200 ++++++++++ crates/openshell-server/src/auth/mod.rs | 1 + crates/openshell-server/src/auth/oidc.rs | 12 +- .../src/auth/sandbox_methods.rs | 22 +- crates/openshell-server/src/grpc/mod.rs | 61 ++++ crates/openshell-server/src/inference.rs | 6 + 13 files changed, 676 insertions(+), 145 deletions(-) create mode 100644 crates/openshell-server-macros/Cargo.toml create mode 100644 crates/openshell-server-macros/src/lib.rs create mode 100644 crates/openshell-server/src/auth/method_authz.rs diff --git a/Cargo.lock b/Cargo.lock index 08e1d052f..92bc18499 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3719,6 +3719,7 @@ dependencies = [ "openshell-policy", "openshell-providers", "openshell-router", + "openshell-server-macros", "petname", "pin-project-lite", "prost", @@ -3752,6 +3753,15 @@ dependencies = [ "x509-parser", ] +[[package]] +name = "openshell-server-macros" +version = "0.0.0" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "openshell-tui" version = "0.0.0" diff --git a/crates/openshell-core/build.rs b/crates/openshell-core/build.rs index 7613c8754..12e79a1dc 100644 --- a/crates/openshell-core/build.rs +++ b/crates/openshell-core/build.rs @@ -40,12 +40,23 @@ fn main() -> Result<(), Box> { collect_proto_files(&proto_root, &mut proto_files)?; proto_files.sort(); + let out_dir = PathBuf::from(env::var("OUT_DIR")?); + let descriptor_path = out_dir.join("openshell_descriptor.bin"); + // Configure tonic-build tonic_build::configure() .build_server(true) .build_client(true) + // Emit a binary FileDescriptorSet so the server can enumerate every + // RPC at runtime (used by the per-handler auth exhaustiveness test). + .file_descriptor_set_path(&descriptor_path) .compile_protos(&proto_files, &[proto_root.as_path()])?; + println!( + "cargo:rustc-env=OPENSHELL_DESCRIPTOR_PATH={}", + descriptor_path.display() + ); + Ok(()) } diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index 2c003f38c..17548ad1a 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -43,3 +43,10 @@ pub const VERSION: &str = match option_env!("OPENSHELL_GIT_VERSION") { Some(v) => v, None => env!("CARGO_PKG_VERSION"), }; + +/// Encoded protobuf `FileDescriptorSet` for every proto in `proto/`. +/// +/// Emitted by `build.rs` via `tonic_build::configure().file_descriptor_set_path(...)`. +/// Used by tests in `openshell-server` to enumerate every RPC and verify that +/// each one has an `#[rpc_auth(...)]` declaration on its handler. +pub const FILE_DESCRIPTOR_SET: &[u8] = include_bytes!(env!("OPENSHELL_DESCRIPTOR_PATH")); diff --git a/crates/openshell-server-macros/Cargo.toml b/crates/openshell-server-macros/Cargo.toml new file mode 100644 index 000000000..f929d43a6 --- /dev/null +++ b/crates/openshell-server-macros/Cargo.toml @@ -0,0 +1,18 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +[package] +name = "openshell-server-macros" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true + +[lib] +proc-macro = true + +[dependencies] +proc-macro2 = "1" +quote = "1" +syn = { version = "2", features = ["full", "extra-traits"] } diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs new file mode 100644 index 000000000..8b2e09b6f --- /dev/null +++ b/crates/openshell-server-macros/src/lib.rs @@ -0,0 +1,343 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Proc macros for declaring per-handler gRPC auth metadata. +//! +//! `#[rpc_authz(service = "...")]` is applied to a tonic service `impl` +//! block. Each method inside the impl carries an `#[rpc_auth(...)]` +//! attribute describing its auth mode, optional Bearer scope, and +//! required role. The macro emits a const `&[MethodAuth]` adjacent to +//! the impl and re-emits the impl block with the per-method +//! `#[rpc_auth]` attributes stripped so other macros (notably +//! `#[tonic::async_trait]`) see a clean impl. +//! +//! Generated code references `crate::auth::method_authz::{MethodAuth, +//! AuthMode, Role}`, so the macro is only intended for use inside the +//! `openshell-server` crate. +//! +//! See `architecture/plans/scope-annotations.md` for the design. + +use proc_macro::TokenStream; +use proc_macro2::Span; +use quote::quote; +use syn::parse::{Parse, ParseStream}; +use syn::{ + Error, Ident, ImplItem, ItemImpl, LitStr, Meta, Result, Token, parse_macro_input, + spanned::Spanned, +}; + +struct AuthzArgs { + service: LitStr, +} + +impl Parse for AuthzArgs { + fn parse(input: ParseStream<'_>) -> Result { + let key: Ident = input.parse()?; + if key != "service" { + return Err(Error::new( + key.span(), + "expected `service = \"\"`", + )); + } + let _eq: Token![=] = input.parse()?; + let service: LitStr = input.parse()?; + Ok(Self { service }) + } +} + +struct RpcAuth { + mode: AuthMode, + scope: Option, + role: Option, +} + +#[derive(Clone, Copy)] +enum AuthMode { + Unauthenticated, + SandboxSecret, + Bearer, + Dual, +} + +#[derive(Clone, Copy)] +enum RoleLit { + Admin, + User, +} + +impl RpcAuth { + fn parse(meta: &Meta) -> Result { + let span = meta.span(); + let list = match meta { + Meta::List(list) => list, + _ => { + return Err(Error::new( + span, + "expected `#[rpc_auth(auth = \"...\", scope = \"...\", role = \"...\")]`", + )); + } + }; + + let mut mode: Option = None; + let mut scope: Option = None; + let mut role: Option = None; + + list.parse_nested_meta(|m| { + let ident = m + .path + .get_ident() + .ok_or_else(|| m.error("expected `auth`, `scope`, or `role`"))?; + + if ident == "auth" { + let value: LitStr = m.value()?.parse()?; + mode = Some(parse_auth_mode(&value)?); + } else if ident == "scope" { + let value: LitStr = m.value()?.parse()?; + scope = Some(value); + } else if ident == "role" { + let value: LitStr = m.value()?.parse()?; + role = Some(parse_role(&value)?); + } else { + return Err(m.error("expected `auth`, `scope`, or `role`")); + } + Ok(()) + })?; + + let Some(mode) = mode else { + return Err(Error::new(span, "`#[rpc_auth]` requires `auth = \"...\"`")); + }; + + match mode { + AuthMode::Unauthenticated | AuthMode::SandboxSecret => { + if let Some(ref s) = scope { + return Err(Error::new( + s.span(), + "`scope` is only valid for `auth = \"bearer\"` or `auth = \"dual\"`", + )); + } + if role.is_some() { + return Err(Error::new( + span, + "`role` is only valid for `auth = \"bearer\"` or `auth = \"dual\"`", + )); + } + } + AuthMode::Bearer | AuthMode::Dual => { + if scope.is_none() { + return Err(Error::new( + span, + "`auth = \"bearer\"` and `auth = \"dual\"` require `scope = \"...\"`", + )); + } + if role.is_none() { + return Err(Error::new( + span, + "`auth = \"bearer\"` and `auth = \"dual\"` require `role = \"...\"`", + )); + } + } + } + + Ok(Self { mode, scope, role }) + } +} + +fn parse_auth_mode(value: &LitStr) -> Result { + match value.value().as_str() { + "unauthenticated" => Ok(AuthMode::Unauthenticated), + "sandbox-secret" => Ok(AuthMode::SandboxSecret), + "bearer" => Ok(AuthMode::Bearer), + "dual" => Ok(AuthMode::Dual), + other => Err(Error::new( + value.span(), + format!( + "invalid auth mode `{other}`; expected one of `unauthenticated`, `sandbox-secret`, `bearer`, `dual`" + ), + )), + } +} + +fn parse_role(value: &LitStr) -> Result { + match value.value().as_str() { + "admin" => Ok(RoleLit::Admin), + "user" => Ok(RoleLit::User), + other => Err(Error::new( + value.span(), + format!("invalid role `{other}`; expected `admin` or `user`"), + )), + } +} + +/// Convert a Rust snake_case method identifier to the tonic PascalCase +/// gRPC method name (`list_sandboxes` → `ListSandboxes`). +fn snake_to_pascal(ident: &str) -> String { + let mut out = String::with_capacity(ident.len()); + let mut upper_next = true; + for c in ident.chars() { + if c == '_' { + upper_next = true; + continue; + } + if upper_next { + out.extend(c.to_uppercase()); + upper_next = false; + } else { + out.push(c); + } + } + out +} + +/// `OpenShell` → `OPEN_SHELL_AUTH_METADATA`. +fn const_ident_from_trait(trait_ident: &Ident) -> Ident { + let name = trait_ident.to_string(); + let mut buf = String::with_capacity(name.len() * 2); + for (i, c) in name.chars().enumerate() { + if c.is_ascii_uppercase() && i != 0 { + buf.push('_'); + } + buf.extend(c.to_uppercase()); + } + buf.push_str("_AUTH_METADATA"); + Ident::new(&buf, trait_ident.span()) +} + +fn trait_ident(item: &ItemImpl) -> Result { + let (_, path, _) = item.trait_.as_ref().ok_or_else(|| { + Error::new( + item.span(), + "`#[rpc_authz]` must be applied to a trait impl (`impl Trait for Type`)", + ) + })?; + path.segments + .last() + .map(|seg| seg.ident.clone()) + .ok_or_else(|| Error::new(path.span(), "could not determine trait identifier")) +} + +#[proc_macro_attribute] +pub fn rpc_authz(args: TokenStream, item: TokenStream) -> TokenStream { + let args = parse_macro_input!(args as AuthzArgs); + let mut item = parse_macro_input!(item as ItemImpl); + + match expand(&args, &mut item) { + Ok(tokens) => tokens.into(), + Err(err) => err.to_compile_error().into(), + } +} + +/// Standalone use of `#[rpc_auth]` is a hard error so that a method +/// missing the impl-level `#[rpc_authz]` macro fails to compile rather +/// than silently dropping its auth declaration. +#[proc_macro_attribute] +pub fn rpc_auth(_args: TokenStream, item: TokenStream) -> TokenStream { + let span = proc_macro2::TokenStream::from(item.clone()) + .into_iter() + .next() + .map_or_else(Span::call_site, |t| t.span()); + let err = Error::new( + span, + "`#[rpc_auth]` only has meaning inside an `#[rpc_authz]` impl block", + ) + .to_compile_error(); + let original: proc_macro2::TokenStream = item.into(); + quote! { #err #original }.into() +} + +fn expand(args: &AuthzArgs, item: &mut ItemImpl) -> Result { + let trait_ident = trait_ident(item)?; + let const_name = const_ident_from_trait(&trait_ident); + let service = args.service.value(); + + let mut entries: Vec = Vec::new(); + let mut seen_paths: Vec = Vec::new(); + + for impl_item in &mut item.items { + let ImplItem::Fn(method) = impl_item else { + continue; + }; + + let mut found: Option = None; + let mut kept = Vec::with_capacity(method.attrs.len()); + for attr in method.attrs.drain(..) { + if attr.path().is_ident("rpc_auth") { + if found.is_some() { + return Err(Error::new( + attr.span(), + "duplicate `#[rpc_auth]` on the same method", + )); + } + found = Some(attr.meta); + } else { + kept.push(attr); + } + } + method.attrs = kept; + + let Some(meta) = found else { + return Err(Error::new( + method.sig.ident.span(), + "method is missing `#[rpc_auth(...)]`; every RPC method must declare its auth metadata", + )); + }; + + let auth = RpcAuth::parse(&meta)?; + let method_path = format!( + "/{}/{}", + service, + snake_to_pascal(&method.sig.ident.to_string()) + ); + + if seen_paths.contains(&method_path) { + return Err(Error::new( + method.sig.ident.span(), + format!("duplicate gRPC method path `{method_path}`"), + )); + } + seen_paths.push(method_path.clone()); + + let mode_tokens = match auth.mode { + AuthMode::Unauthenticated => { + quote! { crate::auth::method_authz::AuthMode::Unauthenticated } + } + AuthMode::SandboxSecret => { + quote! { crate::auth::method_authz::AuthMode::SandboxSecret } + } + AuthMode::Bearer => quote! { crate::auth::method_authz::AuthMode::Bearer }, + AuthMode::Dual => quote! { crate::auth::method_authz::AuthMode::Dual }, + }; + + let scope_tokens = match &auth.scope { + Some(s) => quote! { ::core::option::Option::Some(#s) }, + None => quote! { ::core::option::Option::None }, + }; + + let role_tokens = match auth.role { + Some(RoleLit::Admin) => { + quote! { ::core::option::Option::Some(crate::auth::method_authz::Role::Admin) } + } + Some(RoleLit::User) => { + quote! { ::core::option::Option::Some(crate::auth::method_authz::Role::User) } + } + None => quote! { ::core::option::Option::None }, + }; + + entries.push(quote! { + crate::auth::method_authz::MethodAuth { + path: #method_path, + mode: #mode_tokens, + scope: #scope_tokens, + role: #role_tokens, + } + }); + } + + let entries_count = entries.len(); + let entries_array = quote! { [#(#entries),*] }; + + Ok(quote! { + pub const #const_name: &[crate::auth::method_authz::MethodAuth; #entries_count] = &#entries_array; + + #item + }) +} diff --git a/crates/openshell-server/Cargo.toml b/crates/openshell-server/Cargo.toml index 69319f63a..9f3c1f33b 100644 --- a/crates/openshell-server/Cargo.toml +++ b/crates/openshell-server/Cargo.toml @@ -24,6 +24,7 @@ openshell-ocsf = { path = "../openshell-ocsf" } openshell-policy = { path = "../openshell-policy" } openshell-providers = { path = "../openshell-providers" } openshell-router = { path = "../openshell-router" } +openshell-server-macros = { path = "../openshell-server-macros" } # Kubernetes client (used by the `generate-certs` subcommand) kube = { workspace = true } diff --git a/crates/openshell-server/src/auth/authz.rs b/crates/openshell-server/src/auth/authz.rs index b4aa072d6..1c04b0976 100644 --- a/crates/openshell-server/src/auth/authz.rs +++ b/crates/openshell-server/src/auth/authz.rs @@ -13,122 +13,10 @@ //! authorization is a gateway concern. use super::identity::Identity; +use super::method_authz::{self, Role}; use tonic::Status; use tracing::debug; -/// gRPC methods that require the admin role. -/// All other authenticated methods require the user role. -const ADMIN_METHODS: &[&str] = &[ - // Provider management - "/openshell.v1.OpenShell/CreateProvider", - "/openshell.v1.OpenShell/UpdateProvider", - "/openshell.v1.OpenShell/DeleteProvider", - "/openshell.v1.OpenShell/ConfigureProviderRefresh", - "/openshell.v1.OpenShell/RotateProviderCredential", - "/openshell.v1.OpenShell/DeleteProviderRefresh", - // Global config and policy - "/openshell.v1.OpenShell/UpdateConfig", - // Draft policy approvals - "/openshell.v1.OpenShell/ApproveDraftChunk", - "/openshell.v1.OpenShell/ApproveAllDraftChunks", - "/openshell.v1.OpenShell/RejectDraftChunk", - "/openshell.v1.OpenShell/EditDraftChunk", - "/openshell.v1.OpenShell/UndoDraftChunk", - "/openshell.v1.OpenShell/ClearDraftChunks", - // Cluster inference write - "/openshell.inference.v1.Inference/SetClusterInference", -]; - -/// Exhaustive mapping of Bearer-authenticated gRPC methods to required scopes. -/// Methods not listed here require `openshell:all` when scope enforcement is enabled. -const SCOPED_METHODS: &[(&str, &str)] = &[ - // sandbox:read - ("/openshell.v1.OpenShell/GetSandbox", "sandbox:read"), - ("/openshell.v1.OpenShell/ListSandboxes", "sandbox:read"), - ( - "/openshell.v1.OpenShell/ListSandboxProviders", - "sandbox:read", - ), - ("/openshell.v1.OpenShell/WatchSandbox", "sandbox:read"), - ("/openshell.v1.OpenShell/GetSandboxLogs", "sandbox:read"), - ("/openshell.v1.OpenShell/GetService", "sandbox:read"), - ("/openshell.v1.OpenShell/ListServices", "sandbox:read"), - ( - "/openshell.v1.OpenShell/GetSandboxPolicyStatus", - "sandbox:read", - ), - ( - "/openshell.v1.OpenShell/ListSandboxPolicies", - "sandbox:read", - ), - // sandbox:write - ("/openshell.v1.OpenShell/CreateSandbox", "sandbox:write"), - ("/openshell.v1.OpenShell/DeleteSandbox", "sandbox:write"), - ("/openshell.v1.OpenShell/ExecSandbox", "sandbox:write"), - ("/openshell.v1.OpenShell/ForwardTcp", "sandbox:write"), - ("/openshell.v1.OpenShell/CreateSshSession", "sandbox:write"), - ("/openshell.v1.OpenShell/RevokeSshSession", "sandbox:write"), - ("/openshell.v1.OpenShell/ExposeService", "sandbox:write"), - ("/openshell.v1.OpenShell/DeleteService", "sandbox:write"), - ( - "/openshell.v1.OpenShell/AttachSandboxProvider", - "sandbox:write", - ), - ( - "/openshell.v1.OpenShell/DetachSandboxProvider", - "sandbox:write", - ), - // provider:read - ("/openshell.v1.OpenShell/GetProvider", "provider:read"), - ("/openshell.v1.OpenShell/ListProviders", "provider:read"), - ( - "/openshell.v1.OpenShell/GetProviderRefreshStatus", - "provider:read", - ), - // provider:write - ("/openshell.v1.OpenShell/CreateProvider", "provider:write"), - ("/openshell.v1.OpenShell/UpdateProvider", "provider:write"), - ("/openshell.v1.OpenShell/DeleteProvider", "provider:write"), - ( - "/openshell.v1.OpenShell/ConfigureProviderRefresh", - "provider:write", - ), - ( - "/openshell.v1.OpenShell/RotateProviderCredential", - "provider:write", - ), - ( - "/openshell.v1.OpenShell/DeleteProviderRefresh", - "provider:write", - ), - // config:read - ("/openshell.v1.OpenShell/GetGatewayConfig", "config:read"), - ("/openshell.v1.OpenShell/GetSandboxConfig", "config:read"), - ("/openshell.v1.OpenShell/GetDraftPolicy", "config:read"), - ("/openshell.v1.OpenShell/GetDraftHistory", "config:read"), - // config:write - ("/openshell.v1.OpenShell/UpdateConfig", "config:write"), - ("/openshell.v1.OpenShell/ApproveDraftChunk", "config:write"), - ( - "/openshell.v1.OpenShell/ApproveAllDraftChunks", - "config:write", - ), - ("/openshell.v1.OpenShell/RejectDraftChunk", "config:write"), - ("/openshell.v1.OpenShell/EditDraftChunk", "config:write"), - ("/openshell.v1.OpenShell/UndoDraftChunk", "config:write"), - ("/openshell.v1.OpenShell/ClearDraftChunks", "config:write"), - // inference:read - ( - "/openshell.inference.v1.Inference/GetClusterInference", - "inference:read", - ), - // inference:write - ( - "/openshell.inference.v1.Inference/SetClusterInference", - "inference:write", - ), -]; - const SCOPE_ALL: &str = "openshell:all"; /// Authorization policy configuration. @@ -176,10 +64,12 @@ impl AuthzPolicy { /// (authentication-only mode for providers like GitHub). #[allow(clippy::result_large_err)] pub fn check(&self, identity: &Identity, method: &str) -> Result<(), Status> { - let required = if ADMIN_METHODS.contains(&method) { - &self.admin_role - } else { - &self.user_role + let required = match method_authz::required_role(method) { + Some(Role::Admin) => &self.admin_role, + // Default to user role for unknown methods, matching the + // pre-annotation behavior. The exhaustiveness test ensures + // every real RPC has an explicit declaration. + Some(Role::User) | None => &self.user_role, }; // Empty role name = skip role check for this level (auth-only mode). @@ -218,10 +108,7 @@ impl AuthzPolicy { return Ok(()); } - let required_scope = SCOPED_METHODS - .iter() - .find(|(m, _)| *m == method) - .map_or(SCOPE_ALL, |(_, s)| *s); + let required_scope = method_authz::required_scope(method).unwrap_or(SCOPE_ALL); if identity.scopes.iter().any(|s| s == required_scope) { return Ok(()); diff --git a/crates/openshell-server/src/auth/method_authz.rs b/crates/openshell-server/src/auth/method_authz.rs new file mode 100644 index 000000000..b8e6ae988 --- /dev/null +++ b/crates/openshell-server/src/auth/method_authz.rs @@ -0,0 +1,200 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Aggregated auth metadata for every gRPC method. +//! +//! The per-method tables are generated by `#[rpc_authz]` (see +//! `openshell-server-macros`) and live next to each service's `impl` +//! block. This module merges them and exposes the lookup functions +//! consumed by `authz.rs` (role/scope), `oidc.rs` (unauthenticated +//! check), and `sandbox_methods.rs` (sandbox principal allowlist). + +use crate::grpc::OPEN_SHELL_AUTH_METADATA; +use crate::inference::INFERENCE_AUTH_METADATA; + +/// Per-method auth metadata emitted by `#[rpc_authz]`. +/// +/// Built at compile time and looked up at request-dispatch time. +#[derive(Debug, Clone, Copy)] +pub struct MethodAuth { + /// Canonical gRPC path (`/package.Service/Method`). + pub path: &'static str, + /// Authentication mode for the method. + pub mode: AuthMode, + /// Required OIDC scope on the Bearer path. `None` when the method + /// is unauthenticated or sandbox-secret-only. + pub scope: Option<&'static str>, + /// Required role on the Bearer path. `None` when the method is + /// unauthenticated or sandbox-secret-only. + pub role: Option, +} + +/// How a gRPC method is authenticated. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AuthMode { + /// No authentication required (e.g. health probes). + Unauthenticated, + /// Only callable by a sandbox principal (sandbox-minted JWT). + SandboxSecret, + /// Bearer (OIDC) authentication required. + Bearer, + /// Either sandbox principal or Bearer; scope and role apply on + /// the Bearer path only. + Dual, +} + +/// Coarse role mapping. Maps to the configured `admin_role` / +/// `user_role` names at runtime. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Role { + Admin, + User, +} + +/// All per-service auth tables in one flat list. +/// +/// Add a new service by appending its generated const here. +const SERVICES: &[&[MethodAuth]] = &[OPEN_SHELL_AUTH_METADATA, INFERENCE_AUTH_METADATA]; + +/// Find the auth metadata for `method`, if any. +#[must_use] +pub fn lookup(method: &str) -> Option<&'static MethodAuth> { + for table in SERVICES { + if let Some(entry) = table.iter().find(|m| m.path == method) { + return Some(entry); + } + } + None +} + +/// All registered RPC paths across every service. Used by tests. +#[cfg(test)] +pub fn all_paths() -> impl Iterator { + SERVICES.iter().flat_map(|s| s.iter()).map(|m| m.path) +} + +/// Required Bearer scope for the method, or `None` if scopes don't +/// apply (unauthenticated, sandbox-secret). +#[must_use] +pub fn required_scope(method: &str) -> Option<&'static str> { + lookup(method).and_then(|m| m.scope) +} + +/// Required role for the method on the Bearer path. +#[must_use] +pub fn required_role(method: &str) -> Option { + lookup(method).and_then(|m| m.role) +} + +/// `true` if the method bypasses authentication entirely. +/// +/// Note: this checks the per-method tables only. The prefix-based +/// bypass for gRPC reflection and health probes is layered on top by +/// `oidc::is_unauthenticated_method`. +#[must_use] +pub fn is_unauthenticated(method: &str) -> bool { + matches!( + lookup(method).map(|m| m.mode), + Some(AuthMode::Unauthenticated) + ) +} + +/// `true` if the method is callable by a sandbox principal +/// (sandbox-secret or dual-auth). +#[must_use] +pub fn is_sandbox_callable(method: &str) -> bool { + matches!( + lookup(method).map(|m| m.mode), + Some(AuthMode::SandboxSecret | AuthMode::Dual) + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use prost::Message; + use prost_types::FileDescriptorSet; + + /// Every RPC declared in any proto under `proto/` must have an + /// `#[rpc_auth(...)]` annotation on its handler. This catches: + /// - new RPCs added to a proto but no annotation on the handler + /// - typo'd method names in annotations (path mismatch) + /// - services that were never given an `#[rpc_authz]` impl + #[test] + fn every_proto_rpc_has_an_annotation() { + let set = FileDescriptorSet::decode(openshell_core::FILE_DESCRIPTOR_SET) + .expect("decode descriptor set"); + + let mut missing: Vec = Vec::new(); + + for file in &set.file { + let package = file.package.as_deref().unwrap_or(""); + // Only check services the gateway actually serves. Skip the + // compute-driver, sandbox supervisor, and test protos because + // those are not surfaced through the gateway's gRPC server. + if package != "openshell.v1" && package != "openshell.inference.v1" { + continue; + } + for svc in &file.service { + let svc_name = svc.name.as_deref().unwrap_or(""); + for method in &svc.method { + let method_name = method.name.as_deref().unwrap_or(""); + let path = format!("/{package}.{svc_name}/{method_name}"); + if lookup(&path).is_none() { + missing.push(path); + } + } + } + } + + assert!( + missing.is_empty(), + "RPC methods missing #[rpc_auth] annotation: {missing:?}" + ); + } + + /// Every annotated path must exist as a real RPC in some proto. This + /// catches stale annotations after an RPC is removed or renamed. + #[test] + fn every_annotated_path_matches_a_real_rpc() { + let set = FileDescriptorSet::decode(openshell_core::FILE_DESCRIPTOR_SET) + .expect("decode descriptor set"); + + let mut proto_paths: Vec = Vec::new(); + for file in &set.file { + let package = file.package.as_deref().unwrap_or(""); + for svc in &file.service { + let svc_name = svc.name.as_deref().unwrap_or(""); + for method in &svc.method { + let method_name = method.name.as_deref().unwrap_or(""); + proto_paths.push(format!("/{package}.{svc_name}/{method_name}")); + } + } + } + + let mut stale: Vec<&'static str> = Vec::new(); + for path in all_paths() { + if !proto_paths.iter().any(|p| p == path) { + stale.push(path); + } + } + + assert!( + stale.is_empty(), + "annotated paths that don't match any real proto RPC: {stale:?}" + ); + } + + /// Sanity check: no path appears in more than one service table. + #[test] + fn no_duplicate_paths_across_services() { + let mut seen: Vec<&'static str> = Vec::new(); + for path in all_paths() { + assert!( + !seen.contains(&path), + "duplicate path across tables: {path}" + ); + seen.push(path); + } + } +} diff --git a/crates/openshell-server/src/auth/mod.rs b/crates/openshell-server/src/auth/mod.rs index ca032a006..cbf3b94d9 100644 --- a/crates/openshell-server/src/auth/mod.rs +++ b/crates/openshell-server/src/auth/mod.rs @@ -14,6 +14,7 @@ pub mod guard; mod http; pub mod identity; pub mod k8s_sa; +pub mod method_authz; pub mod oidc; pub mod principal; pub mod sandbox_jwt; diff --git a/crates/openshell-server/src/auth/oidc.rs b/crates/openshell-server/src/auth/oidc.rs index 5e5a23500..bf5490f2a 100644 --- a/crates/openshell-server/src/auth/oidc.rs +++ b/crates/openshell-server/src/auth/oidc.rs @@ -25,18 +25,16 @@ use tokio::sync::RwLock; use tonic::Status; use tracing::{debug, info, warn}; -/// Truly unauthenticated methods — health probes and infrastructure. -const UNAUTHENTICATED_METHODS: &[&str] = &[ - "/openshell.v1.OpenShell/Health", - "/openshell.inference.v1.Inference/Health", -]; - /// Path prefixes that bypass OIDC validation (gRPC reflection, health probes). +/// +/// These are structural bypasses for gRPC infrastructure that doesn't map to a +/// single RPC method. Per-method bypasses (e.g. `Health`) are declared at the +/// handler with `#[rpc_auth(auth = "unauthenticated")]`. const UNAUTHENTICATED_PREFIXES: &[&str] = &["/grpc.reflection.", "/grpc.health."]; /// Returns `true` if the method needs no authentication at all. pub fn is_unauthenticated_method(path: &str) -> bool { - UNAUTHENTICATED_METHODS.contains(&path) + super::method_authz::is_unauthenticated(path) || UNAUTHENTICATED_PREFIXES .iter() .any(|prefix| path.starts_with(prefix)) diff --git a/crates/openshell-server/src/auth/sandbox_methods.rs b/crates/openshell-server/src/auth/sandbox_methods.rs index e03b8eeb6..70eaa58ef 100644 --- a/crates/openshell-server/src/auth/sandbox_methods.rs +++ b/crates/openshell-server/src/auth/sandbox_methods.rs @@ -7,25 +7,13 @@ //! must not authorize user-facing or admin APIs. The router rejects sandbox //! principals for every method outside this supervisor-to-gateway allowlist; //! handlers still perform same-sandbox checks on request bodies. - -/// Methods a `Principal::Sandbox` may invoke. -const ALLOWED_SANDBOX_METHODS: &[&str] = &[ - "/openshell.v1.OpenShell/IssueSandboxToken", - "/openshell.v1.OpenShell/RefreshSandboxToken", - "/openshell.v1.OpenShell/ConnectSupervisor", - "/openshell.v1.OpenShell/RelayStream", - "/openshell.v1.OpenShell/GetSandboxConfig", - "/openshell.v1.OpenShell/GetSandboxProviderEnvironment", - "/openshell.v1.OpenShell/UpdateConfig", - "/openshell.v1.OpenShell/ReportPolicyStatus", - "/openshell.v1.OpenShell/PushSandboxLogs", - "/openshell.v1.OpenShell/SubmitPolicyAnalysis", - "/openshell.v1.OpenShell/GetDraftPolicy", - "/openshell.inference.v1.Inference/GetInferenceBundle", -]; +//! +//! The allowlist is derived from per-handler `#[rpc_auth(...)]` annotations: +//! a method is callable by a sandbox principal when its declared auth mode is +//! `sandbox-secret` or `dual`. pub fn is_sandbox_callable(path: &str) -> bool { - ALLOWED_SANDBOX_METHODS.contains(&path) + super::method_authz::is_sandbox_callable(path) } #[cfg(test)] diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 8538c8658..e9da4ed9b 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -51,6 +51,12 @@ use tokio_stream::wrappers::ReceiverStream; use tonic::{Request, Response, Status}; use crate::ServerState; +// `rpc_auth` is consumed by `#[rpc_authz]` and so looks unused to the +// import-checker. Keep it imported so a missing `#[rpc_authz]` produces a +// "macro misuse" error from the standalone-rpc_auth fallback rather than +// "cannot find attribute `rpc_auth` in this scope". +#[allow(unused_imports)] +use openshell_server_macros::{rpc_auth, rpc_authz}; // --------------------------------------------------------------------------- // Public re-exports @@ -192,8 +198,10 @@ impl OpenShellService { // Trait impl — thin delegation to submodules // --------------------------------------------------------------------------- +#[rpc_authz(service = "openshell.v1.OpenShell")] #[tonic::async_trait] impl OpenShell for OpenShellService { + #[rpc_auth(auth = "unauthenticated")] async fn health( &self, _request: Request, @@ -206,6 +214,7 @@ impl OpenShell for OpenShellService { // --- Sandbox lifecycle --- + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn create_sandbox( &self, request: Request, @@ -215,6 +224,7 @@ impl OpenShell for OpenShellService { type WatchSandboxStream = ReceiverStream>; + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn watch_sandbox( &self, request: Request, @@ -222,6 +232,7 @@ impl OpenShell for OpenShellService { sandbox::handle_watch_sandbox(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn get_sandbox( &self, request: Request, @@ -229,6 +240,7 @@ impl OpenShell for OpenShellService { sandbox::handle_get_sandbox(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn list_sandboxes( &self, request: Request, @@ -236,6 +248,7 @@ impl OpenShell for OpenShellService { sandbox::handle_list_sandboxes(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn list_sandbox_providers( &self, request: Request, @@ -243,6 +256,7 @@ impl OpenShell for OpenShellService { sandbox::handle_list_sandbox_providers(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn attach_sandbox_provider( &self, request: Request, @@ -250,6 +264,7 @@ impl OpenShell for OpenShellService { sandbox::handle_attach_sandbox_provider(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn detach_sandbox_provider( &self, request: Request, @@ -257,6 +272,7 @@ impl OpenShell for OpenShellService { sandbox::handle_detach_sandbox_provider(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn delete_sandbox( &self, request: Request, @@ -268,6 +284,7 @@ impl OpenShell for OpenShellService { type ExecSandboxStream = ReceiverStream>; + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn exec_sandbox( &self, request: Request, @@ -278,6 +295,7 @@ impl OpenShell for OpenShellService { type ForwardTcpStream = Pin> + Send + 'static>>; + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn forward_tcp( &self, request: Request>, @@ -287,6 +305,7 @@ impl OpenShell for OpenShellService { type ExecSandboxInteractiveStream = ReceiverStream>; + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn exec_sandbox_interactive( &self, request: Request>, @@ -296,6 +315,7 @@ impl OpenShell for OpenShellService { // --- SSH sessions --- + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn create_ssh_session( &self, request: Request, @@ -303,6 +323,7 @@ impl OpenShell for OpenShellService { sandbox::handle_create_ssh_session(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn expose_service( &self, request: Request, @@ -310,6 +331,7 @@ impl OpenShell for OpenShellService { service::handle_expose_service(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn get_service( &self, request: Request, @@ -317,6 +339,7 @@ impl OpenShell for OpenShellService { service::handle_get_service(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn list_services( &self, request: Request, @@ -324,6 +347,7 @@ impl OpenShell for OpenShellService { service::handle_list_services(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn delete_service( &self, request: Request, @@ -331,6 +355,7 @@ impl OpenShell for OpenShellService { service::handle_delete_service(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn revoke_ssh_session( &self, request: Request, @@ -340,6 +365,7 @@ impl OpenShell for OpenShellService { // --- Providers --- + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn create_provider( &self, request: Request, @@ -347,6 +373,7 @@ impl OpenShell for OpenShellService { provider::handle_create_provider(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn get_provider( &self, request: Request, @@ -354,6 +381,7 @@ impl OpenShell for OpenShellService { provider::handle_get_provider(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn list_providers( &self, request: Request, @@ -361,6 +389,7 @@ impl OpenShell for OpenShellService { provider::handle_list_providers(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn list_provider_profiles( &self, request: Request, @@ -368,6 +397,7 @@ impl OpenShell for OpenShellService { provider::handle_list_provider_profiles(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn get_provider_profile( &self, request: Request, @@ -375,6 +405,7 @@ impl OpenShell for OpenShellService { provider::handle_get_provider_profile(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn import_provider_profiles( &self, request: Request, @@ -382,6 +413,7 @@ impl OpenShell for OpenShellService { provider::handle_import_provider_profiles(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn lint_provider_profiles( &self, request: Request, @@ -389,6 +421,7 @@ impl OpenShell for OpenShellService { provider::handle_lint_provider_profiles(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn update_provider( &self, request: Request, @@ -396,6 +429,7 @@ impl OpenShell for OpenShellService { provider::handle_update_provider(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn get_provider_refresh_status( &self, request: Request, @@ -403,6 +437,7 @@ impl OpenShell for OpenShellService { provider::handle_get_provider_refresh_status(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn configure_provider_refresh( &self, request: Request, @@ -410,6 +445,7 @@ impl OpenShell for OpenShellService { provider::handle_configure_provider_refresh(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn rotate_provider_credential( &self, request: Request, @@ -417,6 +453,7 @@ impl OpenShell for OpenShellService { provider::handle_rotate_provider_credential(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn delete_provider_refresh( &self, request: Request, @@ -424,6 +461,7 @@ impl OpenShell for OpenShellService { provider::handle_delete_provider_refresh(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn delete_provider( &self, request: Request, @@ -431,6 +469,7 @@ impl OpenShell for OpenShellService { provider::handle_delete_provider(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")] async fn delete_provider_profile( &self, request: Request, @@ -440,6 +479,7 @@ impl OpenShell for OpenShellService { // --- Config / Policy --- + #[rpc_auth(auth = "dual", scope = "config:read", role = "user")] async fn get_sandbox_config( &self, request: Request, @@ -447,6 +487,7 @@ impl OpenShell for OpenShellService { policy::handle_get_sandbox_config(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:read", role = "user")] async fn get_gateway_config( &self, request: Request, @@ -454,6 +495,7 @@ impl OpenShell for OpenShellService { policy::handle_get_gateway_config(&self.state, request).await } + #[rpc_auth(auth = "sandbox-secret")] async fn get_sandbox_provider_environment( &self, request: Request, @@ -461,6 +503,7 @@ impl OpenShell for OpenShellService { policy::handle_get_sandbox_provider_environment(&self.state, request).await } + #[rpc_auth(auth = "dual", scope = "config:write", role = "admin")] async fn update_config( &self, request: Request, @@ -468,6 +511,7 @@ impl OpenShell for OpenShellService { policy::handle_update_config(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn get_sandbox_policy_status( &self, request: Request, @@ -475,6 +519,7 @@ impl OpenShell for OpenShellService { policy::handle_get_sandbox_policy_status(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn list_sandbox_policies( &self, request: Request, @@ -482,6 +527,7 @@ impl OpenShell for OpenShellService { policy::handle_list_sandbox_policies(&self.state, request).await } + #[rpc_auth(auth = "sandbox-secret")] async fn report_policy_status( &self, request: Request, @@ -491,6 +537,7 @@ impl OpenShell for OpenShellService { // --- Sandbox logs --- + #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn get_sandbox_logs( &self, request: Request, @@ -498,6 +545,7 @@ impl OpenShell for OpenShellService { policy::handle_get_sandbox_logs(&self.state, request).await } + #[rpc_auth(auth = "sandbox-secret")] async fn push_sandbox_logs( &self, request: Request>, @@ -507,6 +555,7 @@ impl OpenShell for OpenShellService { // --- Draft policy recommendations --- + #[rpc_auth(auth = "sandbox-secret")] async fn submit_policy_analysis( &self, request: Request, @@ -514,6 +563,7 @@ impl OpenShell for OpenShellService { policy::handle_submit_policy_analysis(&self.state, request).await } + #[rpc_auth(auth = "dual", scope = "config:read", role = "user")] async fn get_draft_policy( &self, request: Request, @@ -521,6 +571,7 @@ impl OpenShell for OpenShellService { policy::handle_get_draft_policy(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:write", role = "admin")] async fn approve_draft_chunk( &self, request: Request, @@ -528,6 +579,7 @@ impl OpenShell for OpenShellService { policy::handle_approve_draft_chunk(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:write", role = "admin")] async fn reject_draft_chunk( &self, request: Request, @@ -535,6 +587,7 @@ impl OpenShell for OpenShellService { policy::handle_reject_draft_chunk(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:write", role = "admin")] async fn approve_all_draft_chunks( &self, request: Request, @@ -542,6 +595,7 @@ impl OpenShell for OpenShellService { policy::handle_approve_all_draft_chunks(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:write", role = "admin")] async fn edit_draft_chunk( &self, request: Request, @@ -549,6 +603,7 @@ impl OpenShell for OpenShellService { policy::handle_edit_draft_chunk(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:write", role = "admin")] async fn undo_draft_chunk( &self, request: Request, @@ -556,6 +611,7 @@ impl OpenShell for OpenShellService { policy::handle_undo_draft_chunk(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:write", role = "admin")] async fn clear_draft_chunks( &self, request: Request, @@ -563,6 +619,7 @@ impl OpenShell for OpenShellService { policy::handle_clear_draft_chunks(&self.state, request).await } + #[rpc_auth(auth = "bearer", scope = "config:read", role = "user")] async fn get_draft_history( &self, request: Request, @@ -572,6 +629,7 @@ impl OpenShell for OpenShellService { // --- Sandbox identity --- + #[rpc_auth(auth = "sandbox-secret")] async fn issue_sandbox_token( &self, request: Request, @@ -579,6 +637,7 @@ impl OpenShell for OpenShellService { auth_rpc::handle_issue_sandbox_token(&self.state, request).await } + #[rpc_auth(auth = "sandbox-secret")] async fn refresh_sandbox_token( &self, request: Request, @@ -591,6 +650,7 @@ impl OpenShell for OpenShellService { type ConnectSupervisorStream = Pin> + Send + 'static>>; + #[rpc_auth(auth = "sandbox-secret")] async fn connect_supervisor( &self, request: Request>, @@ -601,6 +661,7 @@ impl OpenShell for OpenShellService { type RelayStreamStream = Pin> + Send + 'static>>; + #[rpc_auth(auth = "sandbox-secret")] async fn relay_stream( &self, request: Request>, diff --git a/crates/openshell-server/src/inference.rs b/crates/openshell-server/src/inference.rs index 183a80e74..363112d36 100644 --- a/crates/openshell-server/src/inference.rs +++ b/crates/openshell-server/src/inference.rs @@ -12,6 +12,8 @@ use openshell_core::proto::{ }; use openshell_router::config::ResolvedRoute as RouterResolvedRoute; use openshell_router::{ValidationFailureKind, verify_backend_endpoint}; +#[allow(unused_imports)] +use openshell_server_macros::{rpc_auth, rpc_authz}; use prost::Message as _; use std::sync::Arc; use std::time::Duration; @@ -55,8 +57,10 @@ impl ObjectType for InferenceRoute { } } +#[rpc_authz(service = "openshell.inference.v1.Inference")] #[tonic::async_trait] impl Inference for InferenceService { + #[rpc_auth(auth = "sandbox-secret")] async fn get_inference_bundle( &self, request: Request, @@ -71,6 +75,7 @@ impl Inference for InferenceService { .map(Response::new) } + #[rpc_auth(auth = "bearer", scope = "inference:write", role = "admin")] async fn set_cluster_inference( &self, request: Request, @@ -105,6 +110,7 @@ impl Inference for InferenceService { })) } + #[rpc_auth(auth = "bearer", scope = "inference:read", role = "user")] async fn get_cluster_inference( &self, request: Request, From 89a5a2af08bdd13fd22e40d8278df389b6564b09 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Tue, 26 May 2026 15:32:09 -0700 Subject: [PATCH 2/7] refactor(server): rename `sandbox-secret` auth mode to `sandbox` PR #1404 replaced the shared sandbox secret with per-sandbox gateway-minted JWTs. A handler marked `sandbox` now authenticates as a specific `Principal::Sandbox`, not as a holder of a shared credential. Rename `auth = "sandbox-secret"` to `auth = "sandbox"` and `AuthMode::SandboxSecret` to `AuthMode::Sandbox` so the name matches the post-#1404 identity model. Signed-off-by: Mrunal Patel --- crates/openshell-server-macros/src/lib.rs | 14 +++++++------- crates/openshell-server/src/auth/method_authz.rs | 11 ++++++----- .../openshell-server/src/auth/sandbox_methods.rs | 2 +- crates/openshell-server/src/grpc/mod.rs | 16 ++++++++-------- crates/openshell-server/src/inference.rs | 2 +- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs index 8b2e09b6f..b4888062c 100644 --- a/crates/openshell-server-macros/src/lib.rs +++ b/crates/openshell-server-macros/src/lib.rs @@ -54,7 +54,7 @@ struct RpcAuth { #[derive(Clone, Copy)] enum AuthMode { Unauthenticated, - SandboxSecret, + Sandbox, Bearer, Dual, } @@ -108,11 +108,11 @@ impl RpcAuth { }; match mode { - AuthMode::Unauthenticated | AuthMode::SandboxSecret => { + AuthMode::Unauthenticated | AuthMode::Sandbox => { if let Some(ref s) = scope { return Err(Error::new( s.span(), - "`scope` is only valid for `auth = \"bearer\"` or `auth = \"dual\"`", + "`scope` is only valid for `auth = \"bearer\"` or `auth = \"dual\"` (sandbox principals don't carry scopes)", )); } if role.is_some() { @@ -145,13 +145,13 @@ impl RpcAuth { fn parse_auth_mode(value: &LitStr) -> Result { match value.value().as_str() { "unauthenticated" => Ok(AuthMode::Unauthenticated), - "sandbox-secret" => Ok(AuthMode::SandboxSecret), + "sandbox" => Ok(AuthMode::Sandbox), "bearer" => Ok(AuthMode::Bearer), "dual" => Ok(AuthMode::Dual), other => Err(Error::new( value.span(), format!( - "invalid auth mode `{other}`; expected one of `unauthenticated`, `sandbox-secret`, `bearer`, `dual`" + "invalid auth mode `{other}`; expected one of `unauthenticated`, `sandbox`, `bearer`, `dual`" ), )), } @@ -300,8 +300,8 @@ fn expand(args: &AuthzArgs, item: &mut ItemImpl) -> Result { quote! { crate::auth::method_authz::AuthMode::Unauthenticated } } - AuthMode::SandboxSecret => { - quote! { crate::auth::method_authz::AuthMode::SandboxSecret } + AuthMode::Sandbox => { + quote! { crate::auth::method_authz::AuthMode::Sandbox } } AuthMode::Bearer => quote! { crate::auth::method_authz::AuthMode::Bearer }, AuthMode::Dual => quote! { crate::auth::method_authz::AuthMode::Dual }, diff --git a/crates/openshell-server/src/auth/method_authz.rs b/crates/openshell-server/src/auth/method_authz.rs index b8e6ae988..e1db56015 100644 --- a/crates/openshell-server/src/auth/method_authz.rs +++ b/crates/openshell-server/src/auth/method_authz.rs @@ -34,8 +34,9 @@ pub struct MethodAuth { pub enum AuthMode { /// No authentication required (e.g. health probes). Unauthenticated, - /// Only callable by a sandbox principal (sandbox-minted JWT). - SandboxSecret, + /// Only callable by a `Principal::Sandbox` (gateway-minted sandbox JWT). + /// See `auth/sandbox_jwt.rs`. + Sandbox, /// Bearer (OIDC) authentication required. Bearer, /// Either sandbox principal or Bearer; scope and role apply on @@ -99,13 +100,13 @@ pub fn is_unauthenticated(method: &str) -> bool { ) } -/// `true` if the method is callable by a sandbox principal -/// (sandbox-secret or dual-auth). +/// `true` if the method is callable by a `Principal::Sandbox` +/// (`sandbox` or `dual` auth mode). #[must_use] pub fn is_sandbox_callable(method: &str) -> bool { matches!( lookup(method).map(|m| m.mode), - Some(AuthMode::SandboxSecret | AuthMode::Dual) + Some(AuthMode::Sandbox | AuthMode::Dual) ) } diff --git a/crates/openshell-server/src/auth/sandbox_methods.rs b/crates/openshell-server/src/auth/sandbox_methods.rs index 70eaa58ef..76d5e1324 100644 --- a/crates/openshell-server/src/auth/sandbox_methods.rs +++ b/crates/openshell-server/src/auth/sandbox_methods.rs @@ -10,7 +10,7 @@ //! //! The allowlist is derived from per-handler `#[rpc_auth(...)]` annotations: //! a method is callable by a sandbox principal when its declared auth mode is -//! `sandbox-secret` or `dual`. +//! `sandbox` or `dual`. pub fn is_sandbox_callable(path: &str) -> bool { super::method_authz::is_sandbox_callable(path) diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index e9da4ed9b..753816193 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -495,7 +495,7 @@ impl OpenShell for OpenShellService { policy::handle_get_gateway_config(&self.state, request).await } - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn get_sandbox_provider_environment( &self, request: Request, @@ -527,7 +527,7 @@ impl OpenShell for OpenShellService { policy::handle_list_sandbox_policies(&self.state, request).await } - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn report_policy_status( &self, request: Request, @@ -545,7 +545,7 @@ impl OpenShell for OpenShellService { policy::handle_get_sandbox_logs(&self.state, request).await } - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn push_sandbox_logs( &self, request: Request>, @@ -555,7 +555,7 @@ impl OpenShell for OpenShellService { // --- Draft policy recommendations --- - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn submit_policy_analysis( &self, request: Request, @@ -629,7 +629,7 @@ impl OpenShell for OpenShellService { // --- Sandbox identity --- - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn issue_sandbox_token( &self, request: Request, @@ -637,7 +637,7 @@ impl OpenShell for OpenShellService { auth_rpc::handle_issue_sandbox_token(&self.state, request).await } - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn refresh_sandbox_token( &self, request: Request, @@ -650,7 +650,7 @@ impl OpenShell for OpenShellService { type ConnectSupervisorStream = Pin> + Send + 'static>>; - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn connect_supervisor( &self, request: Request>, @@ -661,7 +661,7 @@ impl OpenShell for OpenShellService { type RelayStreamStream = Pin> + Send + 'static>>; - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn relay_stream( &self, request: Request>, diff --git a/crates/openshell-server/src/inference.rs b/crates/openshell-server/src/inference.rs index 363112d36..fd401bda0 100644 --- a/crates/openshell-server/src/inference.rs +++ b/crates/openshell-server/src/inference.rs @@ -60,7 +60,7 @@ impl ObjectType for InferenceRoute { #[rpc_authz(service = "openshell.inference.v1.Inference")] #[tonic::async_trait] impl Inference for InferenceService { - #[rpc_auth(auth = "sandbox-secret")] + #[rpc_auth(auth = "sandbox")] async fn get_inference_bundle( &self, request: Request, From 7b636eb822e92c40ff32056191efea772a177dfc Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Tue, 26 May 2026 15:37:28 -0700 Subject: [PATCH 3/7] fix(server): enforce per-handler AuthMode at the router Addresses review feedback on the per-handler auth-annotation work. - Router-level enforcement of #[rpc_auth] auth mode (HIGH). The previous router only checked is_sandbox_callable() for Principal::Sandbox; user principals still flowed into AuthzPolicy::check() and bypassed the per-handler declaration. A user with `openshell:all` could therefore reach `sandbox`-only handlers like GetSandboxProviderEnvironment, ReportPolicyStatus, PushSandboxLogs, and SubmitPolicyAnalysis even though their annotations said sandbox-only. Adds an is_user_callable() predicate and rejects User principals at the router for `sandbox` / `unauthenticated` methods. - Proc macro now errors on duplicate keys in #[rpc_auth(...)] (LOW). A second `auth`, `scope`, or `role` previously silently overwrote the first value; now it fails to compile. - Regression tests: a unit test for is_user_callable() and a router test that proves a user with admin role + openshell:all cannot reach the nine sandbox-only handlers. Signed-off-by: Mrunal Patel --- crates/openshell-server-macros/src/lib.rs | 9 ++++ .../openshell-server/src/auth/method_authz.rs | 50 +++++++++++++++++ crates/openshell-server/src/multiplex.rs | 53 +++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs index b4888062c..f1981b729 100644 --- a/crates/openshell-server-macros/src/lib.rs +++ b/crates/openshell-server-macros/src/lib.rs @@ -89,12 +89,21 @@ impl RpcAuth { .ok_or_else(|| m.error("expected `auth`, `scope`, or `role`"))?; if ident == "auth" { + if mode.is_some() { + return Err(m.error("`auth` specified more than once")); + } let value: LitStr = m.value()?.parse()?; mode = Some(parse_auth_mode(&value)?); } else if ident == "scope" { + if scope.is_some() { + return Err(m.error("`scope` specified more than once")); + } let value: LitStr = m.value()?.parse()?; scope = Some(value); } else if ident == "role" { + if role.is_some() { + return Err(m.error("`role` specified more than once")); + } let value: LitStr = m.value()?.parse()?; role = Some(parse_role(&value)?); } else { diff --git a/crates/openshell-server/src/auth/method_authz.rs b/crates/openshell-server/src/auth/method_authz.rs index e1db56015..2946fa314 100644 --- a/crates/openshell-server/src/auth/method_authz.rs +++ b/crates/openshell-server/src/auth/method_authz.rs @@ -110,6 +110,21 @@ pub fn is_sandbox_callable(method: &str) -> bool { ) } +/// `true` if the method is callable by a `Principal::User` (`bearer` or +/// `dual` auth mode). +/// +/// Unknown methods return `true` so [`AuthzPolicy::check`] still gets a +/// chance to evaluate role/scope and apply the `openshell:all` fallback — +/// the exhaustiveness test prevents this branch from ever firing for real +/// RPCs, but it remains as defense-in-depth. +#[must_use] +pub fn is_user_callable(method: &str) -> bool { + match lookup(method).map(|m| m.mode) { + Some(AuthMode::Sandbox | AuthMode::Unauthenticated) => false, + Some(AuthMode::Bearer | AuthMode::Dual) | None => true, + } +} + #[cfg(test)] mod tests { use super::*; @@ -198,4 +213,39 @@ mod tests { seen.push(path); } } + + /// User principals must be rejected for `sandbox` and `unauthenticated` + /// methods; the auth-mode check at the router enforces this regardless + /// of role/scope, closing the gap where a token with `openshell:all` + /// could otherwise reach sandbox-only handlers. + #[test] + fn user_callable_matches_auth_mode() { + // Bearer + assert!(is_user_callable("/openshell.v1.OpenShell/ListSandboxes")); + // Dual + assert!(is_user_callable("/openshell.v1.OpenShell/GetSandboxConfig")); + // Sandbox-only + assert!(!is_user_callable( + "/openshell.v1.OpenShell/ReportPolicyStatus" + )); + assert!(!is_user_callable("/openshell.v1.OpenShell/PushSandboxLogs")); + assert!(!is_user_callable( + "/openshell.v1.OpenShell/GetSandboxProviderEnvironment" + )); + assert!(!is_user_callable( + "/openshell.v1.OpenShell/SubmitPolicyAnalysis" + )); + assert!(!is_user_callable( + "/openshell.v1.OpenShell/ConnectSupervisor" + )); + assert!(!is_user_callable("/openshell.v1.OpenShell/RelayStream")); + assert!(!is_user_callable( + "/openshell.inference.v1.Inference/GetInferenceBundle" + )); + // Unauthenticated methods are not "user callable" — they're + // intercepted before principal evaluation. + assert!(!is_user_callable("/openshell.v1.OpenShell/Health")); + // Unknown method falls through to AuthzPolicy::check. + assert!(is_user_callable("/openshell.v1.OpenShell/FutureMethod")); + } } diff --git a/crates/openshell-server/src/multiplex.rs b/crates/openshell-server/src/multiplex.rs index 4fcb3993a..ca4fdccd0 100644 --- a/crates/openshell-server/src/multiplex.rs +++ b/crates/openshell-server/src/multiplex.rs @@ -442,6 +442,11 @@ where match principal { Principal::User(ref user) => { + if !crate::auth::method_authz::is_user_callable(&path) { + return Ok(status_response(tonic::Status::permission_denied( + "this method requires a sandbox principal", + ))); + } if let Some(ref policy) = authz_policy && let Err(status) = policy.check(&user.identity, &path) { @@ -1193,6 +1198,54 @@ mod tests { )); } + /// A user principal — even one carrying `openshell:all` and the + /// admin role — must not reach a `sandbox`-annotated method. The + /// router enforces this from the per-handler auth-mode declarations + /// independent of RBAC. + #[tokio::test] + async fn user_principal_is_denied_on_sandbox_only_methods() { + fn admin_user() -> Principal { + Principal::User(UserPrincipal { + identity: Identity { + subject: "admin".to_string(), + display_name: None, + roles: vec!["openshell-admin".to_string()], + scopes: vec!["openshell:all".to_string()], + provider: IdentityProvider::Oidc, + }, + }) + } + + let policy = AuthzPolicy { + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_enabled: true, + }; + + for path in [ + "/openshell.v1.OpenShell/ReportPolicyStatus", + "/openshell.v1.OpenShell/PushSandboxLogs", + "/openshell.v1.OpenShell/SubmitPolicyAnalysis", + "/openshell.v1.OpenShell/GetSandboxProviderEnvironment", + "/openshell.v1.OpenShell/ConnectSupervisor", + "/openshell.v1.OpenShell/RelayStream", + "/openshell.v1.OpenShell/IssueSandboxToken", + "/openshell.v1.OpenShell/RefreshSandboxToken", + "/openshell.inference.v1.Inference/GetInferenceBundle", + ] { + let mock = Arc::new(MockAuthenticator::returning(Ok(Some(admin_user())))); + let chain = AuthenticatorChain::new(vec![mock]); + let (recorder, seen) = PrincipalRecorder::new(); + let mut router = AuthGrpcRouter::new(recorder, Some(chain), Some(policy.clone())); + + let res = router.call(empty_request(path)).await.unwrap(); + + assert!(seen.lock().unwrap().is_none(), "{path} reached handler"); + // grpc-status=7 (PERMISSION_DENIED). + assert_eq!(grpc_status(&res).as_deref(), Some("7"), "{path}"); + } + } + #[tokio::test] async fn sandbox_principal_is_denied_on_user_and_admin_methods() { for path in [ From 0f541330e0f2448b0b36d87dbc0258f42a04c2e0 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Tue, 26 May 2026 15:47:04 -0700 Subject: [PATCH 4/7] docs(server): finish renaming sandbox-secret to sandbox in method_authz doc comments Signed-off-by: Mrunal Patel --- crates/openshell-server/src/auth/method_authz.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/openshell-server/src/auth/method_authz.rs b/crates/openshell-server/src/auth/method_authz.rs index 2946fa314..71aaee747 100644 --- a/crates/openshell-server/src/auth/method_authz.rs +++ b/crates/openshell-server/src/auth/method_authz.rs @@ -22,10 +22,10 @@ pub struct MethodAuth { /// Authentication mode for the method. pub mode: AuthMode, /// Required OIDC scope on the Bearer path. `None` when the method - /// is unauthenticated or sandbox-secret-only. + /// is `unauthenticated` or `sandbox`-only. pub scope: Option<&'static str>, /// Required role on the Bearer path. `None` when the method is - /// unauthenticated or sandbox-secret-only. + /// `unauthenticated` or `sandbox`-only. pub role: Option, } @@ -75,7 +75,7 @@ pub fn all_paths() -> impl Iterator { } /// Required Bearer scope for the method, or `None` if scopes don't -/// apply (unauthenticated, sandbox-secret). +/// apply (`unauthenticated`, `sandbox`). #[must_use] pub fn required_scope(method: &str) -> Option<&'static str> { lookup(method).and_then(|m| m.scope) From 03128df6da7ac0a8522388340e2228853882be0c Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Wed, 27 May 2026 11:19:32 -0700 Subject: [PATCH 5/7] refactor(server-macros): drop standalone `rpc_auth` stub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stub was a safety net that fired only when a method had `#[rpc_auth(...)]` without an enclosing `#[rpc_authz]`. Triggering it required `rpc_auth` to be imported, which is why both call sites carried `#[allow(unused_imports)] use openshell_server_macros::{rpc_auth, rpc_authz};`. Drop the stub and the unused-import workaround. A missing `#[rpc_authz]` now surfaces as rustc's standard "cannot find attribute `rpc_auth` in this scope" — clear enough, and one fewer import + lint exception. Addresses review comment on PR #1596. Signed-off-by: Mrunal Patel --- crates/openshell-server-macros/src/lib.rs | 19 ------------------- crates/openshell-server/src/grpc/mod.rs | 7 +------ crates/openshell-server/src/inference.rs | 3 +-- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs index f1981b729..d0db25638 100644 --- a/crates/openshell-server-macros/src/lib.rs +++ b/crates/openshell-server-macros/src/lib.rs @@ -18,7 +18,6 @@ //! See `architecture/plans/scope-annotations.md` for the design. use proc_macro::TokenStream; -use proc_macro2::Span; use quote::quote; use syn::parse::{Parse, ParseStream}; use syn::{ @@ -235,24 +234,6 @@ pub fn rpc_authz(args: TokenStream, item: TokenStream) -> TokenStream { } } -/// Standalone use of `#[rpc_auth]` is a hard error so that a method -/// missing the impl-level `#[rpc_authz]` macro fails to compile rather -/// than silently dropping its auth declaration. -#[proc_macro_attribute] -pub fn rpc_auth(_args: TokenStream, item: TokenStream) -> TokenStream { - let span = proc_macro2::TokenStream::from(item.clone()) - .into_iter() - .next() - .map_or_else(Span::call_site, |t| t.span()); - let err = Error::new( - span, - "`#[rpc_auth]` only has meaning inside an `#[rpc_authz]` impl block", - ) - .to_compile_error(); - let original: proc_macro2::TokenStream = item.into(); - quote! { #err #original }.into() -} - fn expand(args: &AuthzArgs, item: &mut ItemImpl) -> Result { let trait_ident = trait_ident(item)?; let const_name = const_ident_from_trait(&trait_ident); diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 753816193..32885a9a9 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -51,12 +51,7 @@ use tokio_stream::wrappers::ReceiverStream; use tonic::{Request, Response, Status}; use crate::ServerState; -// `rpc_auth` is consumed by `#[rpc_authz]` and so looks unused to the -// import-checker. Keep it imported so a missing `#[rpc_authz]` produces a -// "macro misuse" error from the standalone-rpc_auth fallback rather than -// "cannot find attribute `rpc_auth` in this scope". -#[allow(unused_imports)] -use openshell_server_macros::{rpc_auth, rpc_authz}; +use openshell_server_macros::rpc_authz; // --------------------------------------------------------------------------- // Public re-exports diff --git a/crates/openshell-server/src/inference.rs b/crates/openshell-server/src/inference.rs index fd401bda0..f393caab3 100644 --- a/crates/openshell-server/src/inference.rs +++ b/crates/openshell-server/src/inference.rs @@ -12,8 +12,7 @@ use openshell_core::proto::{ }; use openshell_router::config::ResolvedRoute as RouterResolvedRoute; use openshell_router::{ValidationFailureKind, verify_backend_endpoint}; -#[allow(unused_imports)] -use openshell_server_macros::{rpc_auth, rpc_authz}; +use openshell_server_macros::rpc_authz; use prost::Message as _; use std::sync::Arc; use std::time::Duration; From a8f611edffc39035b839f42d4eb182e933ff20bb Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Wed, 27 May 2026 11:22:45 -0700 Subject: [PATCH 6/7] refactor(server-macros): emit fixed `AUTH_METADATA` const per service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous trait-derived const name turned `OpenShell` into `OPEN_SHELL_AUTH_METADATA`, splitting the project name across an underscore. Each impl already lives in its own module (`crate::grpc::`, `crate::inference::`), so the module path is enough to disambiguate between services — a fixed `AUTH_METADATA` name reads more naturally. Aggregator in `auth/method_authz.rs` now references `crate::grpc::AUTH_METADATA` and `crate::inference::AUTH_METADATA` directly. Addresses review comment on PR #1596. Signed-off-by: Mrunal Patel --- crates/openshell-server-macros/src/lib.rs | 23 ++++++++----------- .../openshell-server/src/auth/method_authz.rs | 9 ++++---- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs index d0db25638..456f7c19e 100644 --- a/crates/openshell-server-macros/src/lib.rs +++ b/crates/openshell-server-macros/src/lib.rs @@ -196,19 +196,12 @@ fn snake_to_pascal(ident: &str) -> String { out } -/// `OpenShell` → `OPEN_SHELL_AUTH_METADATA`. -fn const_ident_from_trait(trait_ident: &Ident) -> Ident { - let name = trait_ident.to_string(); - let mut buf = String::with_capacity(name.len() * 2); - for (i, c) in name.chars().enumerate() { - if c.is_ascii_uppercase() && i != 0 { - buf.push('_'); - } - buf.extend(c.to_uppercase()); - } - buf.push_str("_AUTH_METADATA"); - Ident::new(&buf, trait_ident.span()) -} +/// Name of the per-service const emitted alongside the impl block. The +/// service module is what disambiguates between services — every impl +/// lives in its own module (`crate::grpc::AUTH_METADATA`, +/// `crate::inference::AUTH_METADATA`), so a fixed name reads more +/// naturally than `OPEN_SHELL_AUTH_METADATA` / `INFERENCE_AUTH_METADATA`. +const AUTH_METADATA_CONST: &str = "AUTH_METADATA"; fn trait_ident(item: &ItemImpl) -> Result { let (_, path, _) = item.trait_.as_ref().ok_or_else(|| { @@ -235,8 +228,10 @@ pub fn rpc_authz(args: TokenStream, item: TokenStream) -> TokenStream { } fn expand(args: &AuthzArgs, item: &mut ItemImpl) -> Result { + // `trait_ident` is still called for its validation side effect: the + // macro must be applied to a trait impl (`impl Trait for Type`). let trait_ident = trait_ident(item)?; - let const_name = const_ident_from_trait(&trait_ident); + let const_name = Ident::new(AUTH_METADATA_CONST, trait_ident.span()); let service = args.service.value(); let mut entries: Vec = Vec::new(); diff --git a/crates/openshell-server/src/auth/method_authz.rs b/crates/openshell-server/src/auth/method_authz.rs index 71aaee747..ec8dc5bca 100644 --- a/crates/openshell-server/src/auth/method_authz.rs +++ b/crates/openshell-server/src/auth/method_authz.rs @@ -9,9 +9,6 @@ //! consumed by `authz.rs` (role/scope), `oidc.rs` (unauthenticated //! check), and `sandbox_methods.rs` (sandbox principal allowlist). -use crate::grpc::OPEN_SHELL_AUTH_METADATA; -use crate::inference::INFERENCE_AUTH_METADATA; - /// Per-method auth metadata emitted by `#[rpc_authz]`. /// /// Built at compile time and looked up at request-dispatch time. @@ -54,8 +51,10 @@ pub enum Role { /// All per-service auth tables in one flat list. /// -/// Add a new service by appending its generated const here. -const SERVICES: &[&[MethodAuth]] = &[OPEN_SHELL_AUTH_METADATA, INFERENCE_AUTH_METADATA]; +/// Add a new service by appending its module's `AUTH_METADATA` const here. +/// The constant name is fixed by `#[rpc_authz]`; service disambiguation +/// comes from the module path. +const SERVICES: &[&[MethodAuth]] = &[crate::grpc::AUTH_METADATA, crate::inference::AUTH_METADATA]; /// Find the auth metadata for `method`, if any. #[must_use] From cd2669fdffcb992d40db271b615733dd3e59abd7 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Wed, 27 May 2026 12:25:41 -0700 Subject: [PATCH 7/7] docs(server-macros): fix typo in AUTH_METADATA_CONST doc comment OpenShell is one word; reference name in the doc should be OPENSHELL_AUTH_METADATA, not OPEN_SHELL_AUTH_METADATA. Addresses review nit on PR #1596. Signed-off-by: Mrunal Patel --- crates/openshell-server-macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs index 456f7c19e..a698ae662 100644 --- a/crates/openshell-server-macros/src/lib.rs +++ b/crates/openshell-server-macros/src/lib.rs @@ -200,7 +200,7 @@ fn snake_to_pascal(ident: &str) -> String { /// service module is what disambiguates between services — every impl /// lives in its own module (`crate::grpc::AUTH_METADATA`, /// `crate::inference::AUTH_METADATA`), so a fixed name reads more -/// naturally than `OPEN_SHELL_AUTH_METADATA` / `INFERENCE_AUTH_METADATA`. +/// naturally than `OPENSHELL_AUTH_METADATA` / `INFERENCE_AUTH_METADATA`. const AUTH_METADATA_CONST: &str = "AUTH_METADATA"; fn trait_ident(item: &ItemImpl) -> Result {