From 0078c0d2b0554cd8276854f6a4893ccd0ae130a7 Mon Sep 17 00:00:00 2001 From: hmirin Date: Wed, 14 Oct 2026 14:27:47 +0900 Subject: [PATCH] Add read-only device roles `git-e2ee device-add --role read|write|admin` assigns a role. Read-only devices receive generation keys but cannot author accepted transitions, and `device-list` shows each device's role. --- src/main.rs | 49 +++++--- src/manifest.rs | 24 ++++ src/policy.rs | 40 +++++++ src/repository.rs | 11 +- tests/cli_roles.rs | 106 +++++++++++++++++ tests/multi_device.rs | 43 +++++++ tests/read_only_helper.rs | 236 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 495 insertions(+), 14 deletions(-) create mode 100644 tests/cli_roles.rs create mode 100644 tests/read_only_helper.rs diff --git a/src/main.rs b/src/main.rs index 09ed287..69bc0da 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use anyhow::Result; -use clap::{Parser, Subcommand}; +use clap::{Parser, Subcommand, ValueEnum}; use git_remote_e2ee::crypto::{KeyFile, PublicDevice}; use git_remote_e2ee::policy::DeviceRoles; @@ -15,6 +15,23 @@ struct Cli { command: Command, } +#[derive(Clone, Copy, Debug, ValueEnum)] +enum DeviceRole { + Read, + Write, + Admin, +} + +impl From for DeviceRoles { + fn from(role: DeviceRole) -> Self { + match role { + DeviceRole::Read => Self::reader(), + DeviceRole::Write => Self::collaborator(), + DeviceRole::Admin => Self::owner(), + } + } +} + #[derive(Subcommand)] enum Command { Keygen { @@ -78,8 +95,8 @@ enum Command { key: PathBuf, #[arg(long)] device: PathBuf, - #[arg(long)] - admin: bool, + #[arg(long, value_enum, default_value = "write")] + role: DeviceRole, }, DeviceRevoke { #[arg(long, conflicts_with = "remote", required_unless_present = "remote")] @@ -178,16 +195,12 @@ fn main() -> Result<()> { remote, key, device, - admin, + role, } => { let pin = admin_pin_path(&key); let public = PublicDevice::read(&device)?; let device_id = public.device_id.clone(); - let roles = if admin { - DeviceRoles::owner() - } else { - DeviceRoles::collaborator() - }; + let roles = DeviceRoles::from(role); let (manifest, policy) = match (storage, remote) { (Some(storage), None) => { EncryptedRepository::new(FilesystemStorage::new(storage), KeyFile::read(&key)?) @@ -239,11 +252,9 @@ fn main() -> Result<()> { }; for device in devices { println!( - "{} reader={} writer={} admin={} status={}", + "{} role={} status={}", device.public.device_id, - device.roles.reader, - device.roles.writer, - device.roles.administrator, + role_name(&device.roles), if device.active() { "active" } else { "revoked" } ); } @@ -252,6 +263,18 @@ fn main() -> Result<()> { Ok(()) } +fn role_name(roles: &DeviceRoles) -> &'static str { + if roles.administrator { + "admin" + } else if roles.writer { + "write" + } else if roles.reader { + "read" + } else { + "none" + } +} + fn admin_pin_path(key: &std::path::Path) -> PathBuf { let mut value = key.as_os_str().to_owned(); value.push(".admin-state.json"); diff --git a/src/manifest.rs b/src/manifest.rs index 8d535f1..6554e04 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -1122,6 +1122,30 @@ mod tests { assert!(error.contains("does not match its sealed audit entry")); } + #[test] + fn reader_only_device_cannot_sign_an_ordinary_manifest() { + let owner = KeyFile::generate(); + let reader = KeyFile::generate_for_repository(owner.repository_root.clone()).unwrap(); + let (genesis, _) = PolicyState::genesis(&owner).unwrap(); + let mut devices = genesis.body.devices.clone(); + devices.push(DeviceRecord { + public: reader.public_device().unwrap(), + roles: DeviceRoles::reader(), + revoked_at: None, + }); + let (policy, _) = PolicyState::successor(&genesis, devices, &owner).unwrap(); + let generation_key = random_key(); + + // seal_manifest can construct a cryptographically valid signature from + // any key. Verification must still enforce the signer's policy role. + let encrypted = writer_manifest(&reader, &policy, &generation_key); + let opened = open_manifest(&encrypted, &generation_key).unwrap(); + let error = verify_manifest(&opened, &policy, None, &generation_key) + .unwrap_err() + .to_string(); + assert!(error.contains("manifest was not signed by an active writer")); + } + #[test] fn reader_trials_all_envelopes_and_reports_inactive_membership_clearly() { let owner = KeyFile::generate(); diff --git a/src/policy.rs b/src/policy.rs index 8ccceb9..5d51e02 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -22,6 +22,14 @@ pub struct DeviceRoles { } impl DeviceRoles { + pub fn reader() -> Self { + Self { + reader: true, + writer: false, + administrator: false, + } + } + pub fn owner() -> Self { Self { reader: true, @@ -265,6 +273,11 @@ impl PolicyState { .is_some_and(|device| device.active() && device.roles.writer) } + pub fn is_active_reader(&self, device_id: &str) -> bool { + self.device(device_id) + .is_some_and(|device| device.active() && device.roles.reader) + } + pub fn is_active_admin(&self, device_id: &str) -> bool { self.device(device_id) .is_some_and(|device| device.active() && device.roles.administrator) @@ -342,6 +355,33 @@ mod tests { assert!(unauthorized.validate_successor(&parent).is_err()); } + #[test] + fn writers_and_administrators_must_also_be_readers() { + let owner = KeyFile::generate(); + let (parent, _) = PolicyState::genesis(&owner).unwrap(); + for roles in [ + DeviceRoles { + reader: false, + writer: true, + administrator: false, + }, + DeviceRoles { + reader: false, + writer: false, + administrator: true, + }, + ] { + let added = KeyFile::generate_for_repository(owner.repository_root.clone()).unwrap(); + let mut devices = parent.body.devices.clone(); + devices.push(DeviceRecord { + public: added.public_device().unwrap(), + roles, + revoked_at: None, + }); + assert!(PolicyState::successor(&parent, devices, &owner).is_err()); + } + } + #[test] fn genesis_cannot_be_substituted_under_a_known_repository_root() { let expected = KeyFile::generate(); diff --git a/src/repository.rs b/src/repository.rs index ca0a21a..b6e7bfa 100644 --- a/src/repository.rs +++ b/src/repository.rs @@ -349,7 +349,16 @@ impl EncryptedRepository { git::ensure_repository(repo)?; let chain = self.current_chain()?; let current = chain.first().context("manifest chain is empty")?.clone(); - if !current.policy.is_active_writer(&self.key.device_id()?) { + let device_id = self.key.device_id()?; + if !current.policy.is_active_writer(&device_id) { + if current.policy.is_active_reader(&device_id) { + if let Some(remote_name) = remote_name { + bail!( + "this device is read-only for {remote_name}; ask an administrator for the write role" + ) + } + bail!("this device is read-only; ask an administrator for the write role") + } bail!("device is not an active writer") } if let Some(remote_name) = remote_name { diff --git a/tests/cli_roles.rs b/tests/cli_roles.rs new file mode 100644 index 0000000..e19e80b --- /dev/null +++ b/tests/cli_roles.rs @@ -0,0 +1,106 @@ +use std::path::Path; +use std::process::{Command, Output}; + +use git_remote_e2ee::crypto::KeyFile; + +fn run_cli(args: &[String]) -> Output { + Command::new(env!("CARGO_BIN_EXE_git-e2ee")) + .args(args) + .output() + .unwrap() +} + +fn path_arg(path: &Path) -> String { + path.to_string_lossy().into_owned() +} + +fn add_device_args(storage: &Path, key: &Path, public: &Path) -> Vec { + vec![ + "device-add".to_owned(), + "--storage".to_owned(), + path_arg(storage), + "--key".to_owned(), + path_arg(key), + "--device".to_owned(), + path_arg(public), + ] +} + +#[test] +fn device_add_parses_roles_and_device_list_uses_role_names() { + let temporary = tempfile::tempdir().unwrap(); + let storage = temporary.path().join("storage"); + let owner_path = temporary.path().join("owner.key.json"); + let owner = KeyFile::generate(); + owner.write_new(&owner_path).unwrap(); + + let initialized = run_cli(&[ + "init".to_owned(), + "--storage".to_owned(), + path_arg(&storage), + "--key".to_owned(), + path_arg(&owner_path), + ]); + assert!( + initialized.status.success(), + "git-e2ee init: {}", + String::from_utf8_lossy(&initialized.stderr) + ); + + let roles = [ + ("default", None, "write"), + ("reader", Some("read"), "read"), + ("writer", Some("write"), "write"), + ("admin", Some("admin"), "admin"), + ]; + let mut device_ids = Vec::new(); + for (name, selected_role, expected_role) in roles { + let device_key = KeyFile::generate_for_repository(owner.repository_root.clone()).unwrap(); + let public = device_key.public_device().unwrap(); + let public_path = temporary.path().join(format!("{name}.public.json")); + public.write_new(&public_path).unwrap(); + let mut args = add_device_args(&storage, &owner_path, &public_path); + if let Some(role) = selected_role { + args.extend(["--role".to_owned(), role.to_owned()]); + } + let added = run_cli(&args); + assert!( + added.status.success(), + "git-e2ee device-add {name}: {}", + String::from_utf8_lossy(&added.stderr) + ); + device_ids.push((public.device_id, expected_role)); + } + + let mut legacy_args = add_device_args( + &storage, + &owner_path, + &temporary.path().join("default.public.json"), + ); + legacy_args.push("--admin".to_owned()); + let legacy = run_cli(&legacy_args); + assert!(!legacy.status.success()); + assert!(String::from_utf8_lossy(&legacy.stderr).contains("--admin")); + + let listed = run_cli(&[ + "device-list".to_owned(), + "--storage".to_owned(), + path_arg(&storage), + "--key".to_owned(), + path_arg(&owner_path), + ]); + assert!( + listed.status.success(), + "git-e2ee device-list: {}", + String::from_utf8_lossy(&listed.stderr) + ); + let output = String::from_utf8(listed.stdout).unwrap(); + for (device_id, role) in device_ids { + assert!( + output.contains(&format!("{device_id} role={role} status=active")), + "missing role {role} for {device_id} in:\n{output}" + ); + } + let owner_id = owner.device_id().unwrap(); + assert!(output.contains(&format!("{owner_id} role=admin status=active"))); +} diff --git a/tests/multi_device.rs b/tests/multi_device.rs index fc1b370..93bc578 100644 --- a/tests/multi_device.rs +++ b/tests/multi_device.rs @@ -341,6 +341,49 @@ fn non_admin_device_cannot_change_policy() { ); } +#[test] +fn read_only_device_cannot_add_or_revoke_devices() { + let temp = tempfile::tempdir().unwrap(); + let remote = temp.path().join("remote"); + let owner_key = KeyFile::generate(); + let reader_key = KeyFile::generate_for_repository(owner_key.repository_root.clone()).unwrap(); + let candidate_key = + KeyFile::generate_for_repository(owner_key.repository_root.clone()).unwrap(); + let admin_pin = temp.path().join("owner-admin-state.json"); + let owner = EncryptedRepository::new(FilesystemStorage::new(&remote), owner_key); + owner.initialize().unwrap(); + owner.pin_admin_state(&admin_pin).unwrap(); + owner + .add_device( + reader_key.public_device().unwrap(), + DeviceRoles::reader(), + &admin_pin, + ) + .unwrap(); + + let reader = EncryptedRepository::new(FilesystemStorage::new(&remote), reader_key.clone()); + let before = snapshot_files(&remote); + assert!( + reader + .add_device( + candidate_key.public_device().unwrap(), + DeviceRoles::collaborator(), + &admin_pin, + ) + .unwrap_err() + .to_string() + .contains("not an active administrator") + ); + assert!( + reader + .revoke_device(&reader_key.device_id().unwrap(), &admin_pin) + .unwrap_err() + .to_string() + .contains("not an active administrator") + ); + assert_eq!(snapshot_files(&remote), before); +} + fn copy_directory(source: &Path, destination: &Path) { fs::create_dir_all(destination).unwrap(); for entry in fs::read_dir(source).unwrap() { diff --git a/tests/read_only_helper.rs b/tests/read_only_helper.rs new file mode 100644 index 0000000..8e1716e --- /dev/null +++ b/tests/read_only_helper.rs @@ -0,0 +1,236 @@ +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::{Mutex, MutexGuard}; + +use git_remote_e2ee::crypto::KeyFile; +use git_remote_e2ee::policy::DeviceRoles; +use git_remote_e2ee::repository::EncryptedRepository; +use git_remote_e2ee::storage::{FilesystemStorage, GitStorage, Storage}; + +static CACHE_ENV_LOCK: Mutex<()> = Mutex::new(()); + +fn git_output(repo: &Path, args: &[&str], with_helper: bool) -> Output { + let mut command = Command::new("git"); + command.arg("-C").arg(repo).args(args); + if with_helper { + add_helper_to_path(&mut command); + } + command.output().unwrap() +} + +fn git(repo: &Path, args: &[&str], with_helper: bool) -> String { + let output = git_output(repo, args, with_helper); + assert!( + output.status.success(), + "git {:?}: stdout={} stderr={}", + args, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap().trim().to_owned() +} + +fn add_helper_to_path(command: &mut Command) { + let helper = Path::new(env!("CARGO_BIN_EXE_git-remote-e2ee")); + let mut paths = vec![helper.parent().unwrap().to_path_buf()]; + paths.extend(env::split_paths(&env::var_os("PATH").unwrap_or_default())); + command.env("PATH", env::join_paths(paths).unwrap()); +} + +fn initialize_git(repo: &Path) { + fs::create_dir_all(repo).unwrap(); + git(repo, &["init", "-q", "-b", "main"], false); + git(repo, &["config", "user.name", "Read Only Test"], false); + git( + repo, + &["config", "user.email", "read-only@example.invalid"], + false, + ); +} + +fn commit(repo: &Path, contents: &str, message: &str) -> String { + fs::write(repo.join("note.md"), contents).unwrap(); + git(repo, &["add", "note.md"], false); + git(repo, &["commit", "-q", "-m", message], false); + git(repo, &["rev-parse", "HEAD"], false) +} + +fn configure_remote(repo: &Path, name: &str, url: &str, key_path: &Path) { + git(repo, &["remote", "add", name, url], false); + git( + repo, + &[ + "config", + &format!("remote.{name}.e2ee-key"), + key_path.to_str().unwrap(), + ], + false, + ); +} + +fn snapshot_tree(root: &Path) -> Vec<(PathBuf, Vec)> { + fn visit(root: &Path, current: &Path, files: &mut Vec<(PathBuf, Vec)>) { + for entry in fs::read_dir(current).unwrap() { + let entry = entry.unwrap(); + let path = entry.path(); + if entry.file_type().unwrap().is_dir() { + visit(root, &path, files); + } else { + files.push(( + path.strip_prefix(root).unwrap().to_path_buf(), + fs::read(path).unwrap(), + )); + } + } + } + + let mut files = Vec::new(); + visit(root, root, &mut files); + files.sort_by(|left, right| left.0.cmp(&right.0)); + files +} + +fn exercise_read_only_helper( + temporary: &Path, + storage: S, + remote_path: &Path, + remote_url: &str, +) { + fs::create_dir_all(temporary).unwrap(); + let owner_key_path = temporary.join("owner.key.json"); + let reader_key_path = temporary.join("reader.key.json"); + let owner_key = KeyFile::generate(); + let reader_key = KeyFile::generate_for_repository(owner_key.repository_root.clone()).unwrap(); + owner_key.write_new(&owner_key_path).unwrap(); + reader_key.write_new(&reader_key_path).unwrap(); + + let owner = EncryptedRepository::new(storage, owner_key); + owner.initialize().unwrap(); + let admin_pin = temporary.join("owner.admin-state.json"); + owner.pin_admin_state(&admin_pin).unwrap(); + owner + .add_device( + reader_key.public_device().unwrap(), + DeviceRoles::reader(), + &admin_pin, + ) + .unwrap(); + + let source = temporary.join("writer"); + initialize_git(&source); + let first = commit(&source, "initial writer content\n", "initial"); + configure_remote(&source, "private", remote_url, &owner_key_path); + git(&source, &["push", "private", "main"], true); + + let clone = temporary.join("reader-clone"); + let mut clone_command = Command::new("git"); + clone_command + .args([ + "-c", + &format!("e2ee.key={}", reader_key_path.display()), + "clone", + "-q", + remote_url, + ]) + .arg(&clone); + add_helper_to_path(&mut clone_command); + let output = clone_command.output().unwrap(); + assert!( + output.status.success(), + "reader clone failed: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(git(&clone, &["rev-parse", "HEAD"], false), first); + assert_eq!( + fs::read_to_string(clone.join("note.md")).unwrap(), + "initial writer content\n" + ); + + let second = commit(&source, "fetched writer content\n", "second"); + git(&source, &["push", "private", "main"], true); + git(&clone, &["fetch", "origin"], true); + assert_eq!( + git(&clone, &["rev-parse", "refs/remotes/origin/main"], false), + second + ); + + let third = commit(&source, "pulled writer content\n", "third"); + git(&source, &["push", "private", "main"], true); + git(&clone, &["pull", "--ff-only"], true); + assert_eq!(git(&clone, &["rev-parse", "HEAD"], false), third); + assert_eq!( + fs::read_to_string(clone.join("note.md")).unwrap(), + "pulled writer content\n" + ); + + // A fresh clone has no identity; CI runners have no global Git identity. + git(&clone, &["config", "user.name", "Read Only Test"], false); + git( + &clone, + &["config", "user.email", "read-only@example.invalid"], + false, + ); + commit(&clone, "local reader change\n", "reader change"); + let expected_error = + "this device is read-only for origin; ask an administrator for the write role"; + for args in [ + vec!["push", "origin", "HEAD:refs/heads/main"], + vec!["push", "--dry-run", "origin", "HEAD:refs/heads/main"], + vec!["push", "--force", "origin", "HEAD:refs/heads/main"], + ] { + let before = snapshot_tree(remote_path); + let output = git_output(&clone, &args, true); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "push unexpectedly succeeded: {stderr}" + ); + assert!( + stderr.contains(expected_error), + "push error did not explain the read-only role: {stderr}" + ); + assert_eq!( + snapshot_tree(remote_path), + before, + "rejected push changed storage" + ); + } +} + +#[test] +fn read_only_devices_use_both_storage_backends_without_publishing() { + let temporary = tempfile::tempdir().unwrap(); + let filesystem_remote = temporary.path().join("filesystem-remote"); + let filesystem_url = format!("e2ee::{}", filesystem_remote.display()); + exercise_read_only_helper( + temporary.path(), + FilesystemStorage::new(&filesystem_remote), + &filesystem_remote, + &filesystem_url, + ); + + let _cache_lock: MutexGuard<'static, ()> = CACHE_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let cache_dir = temporary.path().join("isolated-carrier-cache"); + // Keep the carrier cache and all test objects outside the real user cache. + unsafe { env::set_var("GIT_REMOTE_E2EE_CACHE_DIR", &cache_dir) }; + + let carrier = temporary.path().join("carrier.git"); + let initialize_output = Command::new("git") + .args(["init", "--bare", "-q"]) + .arg(&carrier) + .output() + .unwrap(); + assert!(initialize_output.status.success()); + let carrier_url = format!("e2ee::git+{}", carrier.display()); + exercise_read_only_helper( + &temporary.path().join("carrier-case"), + GitStorage::open(carrier.to_str().unwrap()).unwrap(), + &carrier, + &carrier_url, + ); +}