crates/gunnar-ui-server/src/reviewer.rs
//! **The signing identity a web approval is made with — the REVIEWER's, not
//! the console's.**
//!
//! # The gap this closes
//!
//! A forge approval is evidence in the sense a signed commit is: `PullRequests.
//! Merge` refuses unless a **verified** approval, checked against gunnar's own
//! key table, names the exact commit the branch is at. The value of "alice
//! approved commit abc" is that a third party can check it later without
//! trusting the server that stored it.
//!
//! Until this module existed, an approval made from the console was signed by
//! the CONSOLE's own key and attributed to the console's principal — see the
//! old [`crate::forge::attribution`]. gunnar verified it (the console's key is
//! registered) and merged, but the approval named the *console*, not the human
//! who clicked Approve. That is exactly the hole an auditor probes in a "signed
//! review": the signature does not name the reviewer, and the
//! `forbid_self_approval` policy — which compares the approval's actor to the
//! pull request's author — can never fire, because every web approval carried
//! the same actor.
//!
//! # What a "passkey-bound key" is here, and why it is not the passkey itself
//!
//! The browser reviewer authenticates with a WebAuthn passkey (an ES256
//! credential). A passkey signs a WebAuthn assertion over a challenge; it
//! **cannot** produce the `sshsig` that gunnar's forge log is made of, and
//! gunnar's key table holds ssh ed25519 keys, not WebAuthn credentials. So the
//! passkey cannot *be* the forge signing key.
//!
//! gunnar's own issuer is X.509/mTLS (`gunnar issuer create/trust`) — the
//! transport's client-auth PKI, a different mechanism again from the sshsig
//! key table a forge approval is verified against. Neither the passkey nor an
//! issuer cert can sign a forge event.
//!
//! What fits gunnar's *existing* forge model is the key table itself: each
//! browser reviewer gets their **own gunnar principal** and their **own
//! ed25519 signing key registered under it**. The key is minted server-side and
//! held by the console (sealed alongside the session-cookie key in the state
//! store) — the "convenient, weaker form" gunnar's issuer docs already bless
//! (`gunnar principal add alice --server-generated-key`). It is *bound to the
//! passkey* in that the console signs with reviewer `alice`'s key only inside a
//! request `alice`'s passkey session authenticated, and never otherwise.
//!
//! # Honest about the trust it does and does not buy
//!
//! A human holding their own offline ssh key is stronger: the console never
//! sees the private half, so it cannot manufacture their approval. Here the
//! console custodies the key, so a compromised console could. What this DOES
//! buy, and the old console did not:
//!
//! * the approval **names the reviewer** and verifies as *their* principal
//! against the key table at merge — the audit question is answerable;
//! * `forbid_self_approval` **works**: an author's own web approval now carries
//! the author's principal and is refused at merge, where before every web
//! approval was the console's and the rule was dead;
//! * a reviewer with no provisioned key is **disclosed**, never silently signed
//! as the console (see [`crate::api::prs::approve`]).
//!
//! So this is strictly more evidence than the console-signs-everything path it
//! replaces, in gunnar's own key model, with the weakening stated rather than
//! hidden.
use std::collections::HashMap;
use std::sync::Arc;
use tokio::sync::Mutex;
use crate::grpc::Gunnar;
use crate::pb;
use crate::signer::Signer;
use crate::state::{StateError, StateStore};
/// A reviewer's signing identity: the gunnar principal their approval is
/// attributed to, and a control-plane client that signs **both** the call
/// credential and the forge event with THEIR key.
///
/// Because [`ReviewerIdentity::gunnar`] signs the gRPC credential with the
/// reviewer's key, `Forge.Append` sees the reviewer's principal on the
/// connection and accepts an event whose `actor` is that same principal —
/// which a call signed by the console's key never could, since `Append`
/// refuses an event filed in a principal other than the caller's.
pub struct ReviewerIdentity {
/// The identity-table principal the approval is attributed to and verified as.
pub principal_id: String,
/// The `SHA256:…` fingerprint of the reviewer's registered key.
pub fingerprint: String,
/// A client that acts AS the reviewer on the control plane.
pub gunnar: Gunnar,
}
/// Something went wrong establishing a reviewer's signing identity.
#[derive(Debug)]
pub enum ReviewerError {
/// The state store could not be read or written.
State(StateError),
/// A generated or stored key was not usable.
Signer(crate::signer::SignerError),
/// gunnar refused a provisioning call.
Gunnar(String),
/// A per-user control-plane client could not be built.
Client(String),
}
impl std::fmt::Display for ReviewerError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ReviewerError::State(e) => write!(f, "the reviewer key store: {e}"),
ReviewerError::Signer(e) => write!(f, "the reviewer's signing key: {e}"),
ReviewerError::Gunnar(m) => write!(f, "Gunnar refused to provision the reviewer: {m}"),
ReviewerError::Client(m) => write!(f, "the reviewer's control-plane client: {m}"),
}
}
}
impl std::error::Error for ReviewerError {}
/// Mints and hands out per-reviewer signing identities.
///
/// One per console. Cheap to share behind an [`Arc`]: the state store is a
/// handle and the endpoint is a string.
pub struct ReviewerKeys {
/// gunnar's control-plane endpoint, to build a per-user client from.
endpoint: String,
/// Where per-user private keys and principal ids are persisted, sealed by
/// the same store that holds the session-cookie key.
store: StateStore,
/// Serialises provisioning so two racing first-approvals by one new
/// reviewer create exactly one principal and one key.
provisioning: Mutex<()>,
/// A tiny cache so a warm reviewer does not re-read the store and rebuild a
/// client on every approval. Keyed by web user name.
warm: Mutex<HashMap<String, Arc<Cached>>>,
}
struct Cached {
principal_id: String,
fingerprint: String,
signer: Arc<Signer>,
}
/// The state-store key a reviewer's identity is filed under.
///
/// Namespaced so it cannot collide with `session_key` or a passkey record.
fn store_key(user: &str) -> String {
format!("reviewer-identity:{user}")
}
/// The gunnar principal name a web reviewer is given.
///
/// Prefixed and sanitised: the prefix keeps a browser identity clearly distinct
/// from a CLI principal or the console's own (`gunnar-ui`/`uitest-console`)
/// principal of the same human name, and gunnar makes an account namespace of
/// this name, so it has to be a plain token. Two web users whose names sanitise
/// to the same token would collide; the console's user names are already simple
/// identifiers, so this stays a documented edge rather than a live risk.
pub fn principal_name(user: &str) -> String {
let sanitised: String = user
.chars()
.map(|c| {
if c.is_ascii_alphanumeric() || c == '-' || c == '_' {
c.to_ascii_lowercase()
} else {
'-'
}
})
.collect();
let trimmed = sanitised.trim_matches('-');
format!(
"console-{}",
if trimmed.is_empty() { "user" } else { trimmed }
)
}
impl ReviewerKeys {
/// Build over gunnar's endpoint and the console's state store.
pub fn new(endpoint: impl Into<String>, store: StateStore) -> ReviewerKeys {
ReviewerKeys {
endpoint: endpoint.into(),
store,
provisioning: Mutex::new(()),
warm: Mutex::new(HashMap::new()),
}
}
/// The signing identity a reviewer already has, or `None` if none is
/// provisioned yet.
///
/// This never mints one — it is the read the honest-fallback path uses to
/// decide whether it can sign as the reviewer at all.
pub async fn existing(&self, user: &str) -> Result<Option<ReviewerIdentity>, ReviewerError> {
if let Some(cached) = self.warm.lock().await.get(user).cloned() {
return Ok(Some(self.identity_from(&cached)?));
}
let Some(bytes) = self
.store
.get(&store_key(user))
.map_err(ReviewerError::State)?
else {
return Ok(None);
};
let cached = Arc::new(self.decode(&bytes)?);
self.warm
.lock()
.await
.insert(user.to_owned(), cached.clone());
Ok(Some(self.identity_from(&cached)?))
}
/// The signing identity a reviewer has, minting one on first use.
///
/// `admin` is the console's own admin client — the only credential that may
/// write the key table. Provisioning is three calls: create the
/// principal, register the freshly minted key under it, and grant it Write
/// (which `Forge.Append` and `PullRequests.Create` both require so the
/// reviewer can act as themselves). The private key is persisted only after
/// all three succeed, so a half-provisioned reviewer is retried rather than
/// remembered.
pub async fn ensure(
&self,
user: &str,
admin: &Gunnar,
) -> Result<ReviewerIdentity, ReviewerError> {
if let Some(identity) = self.existing(user).await? {
return Ok(identity);
}
let _guard = self.provisioning.lock().await;
// Re-check under the lock: another request may have provisioned this
// reviewer while we waited for it.
if let Some(identity) = self.existing(user).await? {
return Ok(identity);
}
let private = mint_ed25519().map_err(ReviewerError::Signer)?;
let signer = Signer::from_openssh(&private).map_err(ReviewerError::Signer)?;
let fingerprint = signer.fingerprint().to_owned();
let public = signer.public_key_openssh();
let principal_id = self.provision(admin, user, &public).await?;
// Persist last: principal_id + '\n' + the multi-line OpenSSH private
// key. Split on the first newline reads it back.
let record = format!("{principal_id}\n{private}");
self.store
.put(&store_key(user), record.as_bytes())
.map_err(ReviewerError::State)?;
let cached = Arc::new(Cached {
principal_id,
fingerprint,
signer: Arc::new(signer),
});
self.warm
.lock()
.await
.insert(user.to_owned(), cached.clone());
self.identity_from(&cached)
}
/// Create the principal, register the key, grant Write.
async fn provision(
&self,
admin: &Gunnar,
user: &str,
public_key: &str,
) -> Result<String, ReviewerError> {
let name = principal_name(user);
let principal_id = match admin
.access()
.add_principal(pb::AddPrincipalRequest {
name: name.clone(),
kind: pb::PrincipalKind::User as i32,
// Empty: the account namespace is the name. The
// `AlreadyExists` arm below depends on it — a reviewer
// provisioned twice must collide with itself.
id: String::new(),
})
.await
{
Ok(response) => response
.into_inner()
.principal
.map(|p| p.id)
.unwrap_or_default(),
// **Already there, and NOT adopted** (owner, 2026-10-04). A
// principal this console did not just create is somebody's — an
// employee an account admin enrolled under this name, a principal
// an operator made by hand, or an earlier reviewer whose key the
// console no longer holds. Adopting it by its NAME would add a key
// this console holds the private half of and grant the principal
// server-wide Write: a server-Write identity minted on a name
// anybody who can enroll a principal can choose. So the
// provisioning stops and says why; gunnar keeps whatever that
// principal already was.
Err(status) if status.code() == tonic::Code::AlreadyExists => {
return Err(ReviewerError::Gunnar(format!(
"a principal named {name:?} already exists at Gunnar and this console did not \
make it, so it will not take it over as {user}'s reviewer identity: {}",
status.message()
)));
}
Err(status) => return Err(ReviewerError::Gunnar(status.to_string())),
};
if principal_id.is_empty() {
return Err(ReviewerError::Gunnar(format!(
"Gunnar returned no principal id for {name:?}"
)));
}
admin
.access()
.add_key(pb::AddKeyRequest {
principal_id: principal_id.clone(),
openssh_public_key: public_key.to_owned(),
comment: format!("gunnar-ui reviewer {user}"),
expires_at_unix_ms: None,
// First enrolment: the principal was created a few lines up
// and holds no key to attest with, which is the case gunnar
// exempts. On the `AlreadyExists` path it may hold one, and
// then gunnar refuses and says so.
attestation: None,
})
.await
.map_err(|s| ReviewerError::Gunnar(s.to_string()))?;
// Server-wide Write. A web operator can already effect any write through
// the console's own admin credential; giving THEIR principal Write is
// not an escalation of that reach — it is what makes the write
// attributable to them instead of laundered through the console. Write,
// not Admin: a reviewer signs reviews and opens pull requests, they do
// not manage the key table.
admin
.access()
.grant(pb::GrantRequest {
principal_id: principal_id.clone(),
namespace: String::new(),
repo: String::new(),
level: pb::GrantLevel::Write as i32,
})
.await
.map_err(|s| ReviewerError::Gunnar(s.to_string()))?;
Ok(principal_id)
}
fn decode(&self, bytes: &[u8]) -> Result<Cached, ReviewerError> {
let text = String::from_utf8_lossy(bytes);
let (principal_id, private) = text
.split_once('\n')
.ok_or_else(|| ReviewerError::Gunnar("a stored reviewer record has no key".into()))?;
let signer = Signer::from_openssh(private).map_err(ReviewerError::Signer)?;
Ok(Cached {
principal_id: principal_id.to_owned(),
fingerprint: signer.fingerprint().to_owned(),
signer: Arc::new(signer),
})
}
fn identity_from(&self, cached: &Cached) -> Result<ReviewerIdentity, ReviewerError> {
let gunnar = Gunnar::connect_lazy(&self.endpoint, cached.signer.clone())
.map_err(ReviewerError::Client)?;
Ok(ReviewerIdentity {
principal_id: cached.principal_id.clone(),
fingerprint: cached.fingerprint.clone(),
gunnar,
})
}
}
/// Mint a fresh ed25519 key, as an OpenSSH private-key PEM.
///
/// `ssh-key` is built here with `ed25519`, `std` and `rand_core`, so this is
/// the same crate [`Signer`] loads with — no second key type, no openssl.
///
/// `pub(crate)` for [`crate::tenant`], which mints the same kind of key for a
/// different principal. One minting function, so there is one answer to "what
/// does this console generate".
pub(crate) fn mint_ed25519() -> Result<String, crate::signer::SignerError> {
let key = ssh_key::PrivateKey::random(&mut rand::rand_core::UnwrapErr(rand::rngs::SysRng), ssh_key::Algorithm::Ed25519)
.map_err(crate::signer::SignerError::Sign)?;
Ok(key
.to_openssh(ssh_key::LineEnding::LF)
.map_err(crate::signer::SignerError::Sign)?
.to_string())
}
#[cfg(test)]
mod tests {
use super::*;
/// A reviewer's minted key is a usable signer, and — the red-first heart of
/// this module — its fingerprint is **not** the console's. If a web approval
/// were still signed by the console, the two would be equal and this
/// assertion would fail.
#[test]
fn a_reviewer_key_is_the_reviewers_and_not_the_consoles() {
let console = Signer::from_openssh(mint_ed25519().expect("mint console")).expect("console");
let reviewer =
Signer::from_openssh(mint_ed25519().expect("mint reviewer")).expect("reviewer");
assert_ne!(
reviewer.fingerprint(),
console.fingerprint(),
"a reviewer's key must be distinct from the console's, or the approval names the \
console after all"
);
}
/// An approval built for a reviewer verifies under the REVIEWER's key and
/// **not** under the console's — the exact check `PullRequests.Merge` makes
/// against the key table, one layer down, and the red is the console key.
#[test]
fn an_approval_signed_for_a_reviewer_verifies_as_them_not_the_console() {
use crate::forge::{Event, ForgeEvent, FORGE_NAMESPACE};
let console = Signer::from_openssh(mint_ed25519().expect("mint")).expect("console");
let reviewer = Signer::from_openssh(mint_ed25519().expect("mint")).expect("reviewer");
let event = ForgeEvent {
repo: "alice/uitest".into(),
pull: 1,
actor: principal_name("bob"),
at_unix_ms: 1_700_000_000_000,
event: Event::Approved {
commit: "a".repeat(40),
},
};
let (fingerprint, pem) = reviewer.sign_forge_event(&event).expect("sign");
assert_eq!(fingerprint, reviewer.fingerprint());
assert_ne!(
fingerprint,
console.fingerprint(),
"the approval must be signed by the reviewer's key, not the console's"
);
let reviewer_key =
ssh_key::PublicKey::from_openssh(&reviewer.public_key_openssh()).expect("reviewer key");
let console_key =
ssh_key::PublicKey::from_openssh(&console.public_key_openssh()).expect("console key");
let signature = ssh_key::SshSig::from_pem(pem.as_bytes()).expect("armour");
assert!(
reviewer_key
.verify(FORGE_NAMESPACE, &event.signing_bytes(), &signature)
.is_ok(),
"the reviewer's own key must verify the approval"
);
assert!(
console_key
.verify(FORGE_NAMESPACE, &event.signing_bytes(), &signature)
.is_err(),
"the console's key must NOT verify a reviewer's approval — that equality was the gap"
);
}
/// A gunnar where `console-bob` ALREADY exists — an employee an account
/// admin enrolled under that name — and which records what it is asked.
#[derive(Clone, Default)]
struct Taken {
heard: std::sync::Arc<std::sync::Mutex<Vec<String>>>,
}
#[tonic::async_trait]
impl pb::access_server::Access for Taken {
async fn add_principal(
&self,
request: tonic::Request<pb::AddPrincipalRequest>,
) -> Result<tonic::Response<pb::AddPrincipalResponse>, tonic::Status> {
let name = request.into_inner().name;
self.heard.lock().expect("heard").push(format!("AddPrincipal {name}"));
Err(tonic::Status::already_exists(format!("principal {name} already exists")))
}
async fn list_principals(
&self,
_: tonic::Request<pb::ListPrincipalsRequest>,
) -> Result<tonic::Response<pb::ListPrincipalsResponse>, tonic::Status> {
self.heard.lock().expect("heard").push("ListPrincipals".to_owned());
Ok(tonic::Response::new(pb::ListPrincipalsResponse {
principals: vec![pb::Principal {
id: "console-bob".to_owned(),
name: "console-bob".to_owned(),
kind: pb::PrincipalKind::User as i32,
created_at_unix_ms: 1,
}],
next_page_token: String::new(),
..Default::default()
}))
}
async fn add_key(
&self,
request: tonic::Request<pb::AddKeyRequest>,
) -> Result<tonic::Response<pb::AddKeyResponse>, tonic::Status> {
let principal = request.into_inner().principal_id;
self.heard.lock().expect("heard").push(format!("AddKey {principal}"));
Ok(tonic::Response::new(pb::AddKeyResponse { key: None }))
}
async fn grant(
&self,
request: tonic::Request<pb::GrantRequest>,
) -> Result<tonic::Response<pb::GrantResponse>, tonic::Status> {
let principal = request.into_inner().principal_id;
self.heard.lock().expect("heard").push(format!("Grant {principal}"));
Ok(tonic::Response::new(pb::GrantResponse { grant: None }))
}
}
/// ★ **A principal this console did not create is never adopted by its
/// name** (owner, 2026-10-04). Red before this row: on `AlreadyExists` the
/// provisioning looked the name up, added a key this console holds the
/// private half of, and granted the principal server-wide Write — so an
/// employee enrolled as `console-bob` became a server-Write identity the
/// moment an operator named bob approved a pull request. gunnar must be
/// asked for no key and no grant, and nothing is stored.
#[tokio::test]
async fn an_existing_principal_is_not_adopted_as_a_reviewer_by_its_name() {
let taken = Taken::default();
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
.await
.expect("a loopback port");
let addr = listener.local_addr().expect("its address");
let serving = taken.clone();
tokio::spawn(async move {
tonic::transport::Server::builder()
.add_service(pb::access_server::AccessServer::new(serving))
.serve_with_incoming(tokio_stream::wrappers::TcpListenerStream::new(listener))
.await
});
let endpoint = format!("http://{addr}");
let dir = tempfile::tempdir().expect("a temp dir");
let store = StateStore::open(dir.path().join("state.redb")).expect("a store");
let keys = ReviewerKeys::new(endpoint.clone(), store);
let console = Gunnar::connect_lazy(
&endpoint,
std::sync::Arc::new(
Signer::from_openssh(mint_ed25519().expect("mint")).expect("console"),
),
)
.expect("a handle");
let refused = keys.ensure("bob", &console).await;
assert!(refused.is_err(), "an existing principal was adopted as bob's reviewer");
let heard = taken.heard.lock().expect("heard").clone();
assert!(
!heard.iter().any(|c| c.starts_with("AddKey") || c.starts_with("Grant")),
"gunnar was asked to key or grant a principal this console did not make: {heard:?}"
);
assert!(
keys.existing("bob").await.expect("the store reads").is_none(),
"a reviewer identity was stored for a principal this console did not make"
);
}
#[test]
fn a_principal_name_is_prefixed_sanitised_and_never_empty() {
assert_eq!(principal_name("bob"), "console-bob");
assert_eq!(principal_name("Kim.Lee"), "console-kim-lee");
assert_eq!(principal_name(" "), "console-user");
// The prefix keeps it off a CLI principal or the console's own name.
assert!(principal_name("alice").starts_with("console-"));
}
}