Skip to content

Commit 3f520dd

Browse files
authored
feat(server): declare gRPC auth (mode + scope + role) at the handler, enforce at the router (#1596)
* 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 <mrunalp@gmail.com> * 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 <mrunalp@gmail.com> * 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 <mrunalp@gmail.com> * docs(server): finish renaming sandbox-secret to sandbox in method_authz doc comments Signed-off-by: Mrunal Patel <mrunalp@gmail.com> * refactor(server-macros): drop standalone `rpc_auth` stub 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 <mrunalp@gmail.com> * refactor(server-macros): emit fixed `AUTH_METADATA` const per service 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 <mrunalp@gmail.com> * 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 <mrunalp@gmail.com> --------- Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
1 parent dc1f098 commit 3f520dd

14 files changed

Lines changed: 758 additions & 145 deletions

File tree

Cargo.lock

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/openshell-core/build.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,23 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
4040
collect_proto_files(&proto_root, &mut proto_files)?;
4141
proto_files.sort();
4242

43+
let out_dir = PathBuf::from(env::var("OUT_DIR")?);
44+
let descriptor_path = out_dir.join("openshell_descriptor.bin");
45+
4346
// Configure tonic-build
4447
tonic_build::configure()
4548
.build_server(true)
4649
.build_client(true)
50+
// Emit a binary FileDescriptorSet so the server can enumerate every
51+
// RPC at runtime (used by the per-handler auth exhaustiveness test).
52+
.file_descriptor_set_path(&descriptor_path)
4753
.compile_protos(&proto_files, &[proto_root.as_path()])?;
4854

55+
println!(
56+
"cargo:rustc-env=OPENSHELL_DESCRIPTOR_PATH={}",
57+
descriptor_path.display()
58+
);
59+
4960
Ok(())
5061
}
5162

crates/openshell-core/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,10 @@ pub const VERSION: &str = match option_env!("OPENSHELL_GIT_VERSION") {
4343
Some(v) => v,
4444
None => env!("CARGO_PKG_VERSION"),
4545
};
46+
47+
/// Encoded protobuf `FileDescriptorSet` for every proto in `proto/`.
48+
///
49+
/// Emitted by `build.rs` via `tonic_build::configure().file_descriptor_set_path(...)`.
50+
/// Used by tests in `openshell-server` to enumerate every RPC and verify that
51+
/// each one has an `#[rpc_auth(...)]` declaration on its handler.
52+
pub const FILE_DESCRIPTOR_SET: &[u8] = include_bytes!(env!("OPENSHELL_DESCRIPTOR_PATH"));
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
[package]
5+
name = "openshell-server-macros"
6+
version.workspace = true
7+
edition.workspace = true
8+
rust-version.workspace = true
9+
license.workspace = true
10+
repository.workspace = true
11+
12+
[lib]
13+
proc-macro = true
14+
15+
[dependencies]
16+
proc-macro2 = "1"
17+
quote = "1"
18+
syn = { version = "2", features = ["full", "extra-traits"] }

0 commit comments

Comments
 (0)