From d32af594e46b31bfedffd2b4da7f3e257775bbbd Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 20 Jun 2026 17:19:52 -0400 Subject: [PATCH] feat(server): grant-scope org attachment write paths in pre-receive hook --- Cargo.lock | 2 +- crates/relicario-server/Cargo.toml | 2 +- crates/relicario-server/src/lib.rs | 20 +++++++++++- crates/relicario-server/tests/org_hook.rs | 40 +++++++++++++++++++++++ docs/SECURITY.md | 24 +++++++++++--- 5 files changed, 81 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7b2c20..4eab0c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2220,7 +2220,7 @@ dependencies = [ [[package]] name = "relicario-server" -version = "0.1.0" +version = "0.1.1" dependencies = [ "anyhow", "assert_cmd", diff --git a/crates/relicario-server/Cargo.toml b/crates/relicario-server/Cargo.toml index 97d449b..179e304 100644 --- a/crates/relicario-server/Cargo.toml +++ b/crates/relicario-server/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "relicario-server" -version = "0.1.0" +version = "0.1.1" edition = "2021" description = "Pre-receive Git hook for relicario password manager" license = "GPL-3.0-or-later" diff --git a/crates/relicario-server/src/lib.rs b/crates/relicario-server/src/lib.rs index 97148a7..f5dc9ff 100644 --- a/crates/relicario-server/src/lib.rs +++ b/crates/relicario-server/src/lib.rs @@ -6,7 +6,8 @@ pub enum PathClass { /// `members.json`, `collections.json`, `org.json` — only Owner/Admin may write. Protected, - /// `items//.enc` — writer must hold a grant for ``. + /// `items//.enc` and `attachments///.enc` — + /// writer must hold a grant for ``. Item { collection: String }, /// `keys/.enc`, `manifest.enc`, `.gitignore`, etc. — gated only by the /// per-commit signature check (signer must be a current member). @@ -42,6 +43,23 @@ pub fn classify_path(path: &str) -> PathClass { return PathClass::Item { collection: slug.to_string() }; } + if let Some(rest) = path.strip_prefix("attachments/") { + // Expect exactly: //.enc → three segments. + let segments: Vec<&str> = rest.split('/').collect(); + if segments.len() != 3 { + return PathClass::Rejected( + "attachments path must be attachments///.enc".to_string()); + } + let slug = segments[0]; + if slug.is_empty() { + return PathClass::Rejected("empty collection slug in attachments path".to_string()); + } + if slug.contains('.') { + return PathClass::Rejected(format!("invalid collection slug: {:?}", slug)); + } + return PathClass::Item { collection: slug.to_string() }; + } + PathClass::Unrestricted } diff --git a/crates/relicario-server/tests/org_hook.rs b/crates/relicario-server/tests/org_hook.rs index 7c306bf..48b1475 100644 --- a/crates/relicario-server/tests/org_hook.rs +++ b/crates/relicario-server/tests/org_hook.rs @@ -79,3 +79,43 @@ fn extract_schema_version_errors_on_missing_field() { fn extract_schema_version_errors_on_garbage() { assert!(extract_schema_version("not json").is_err()); } + +#[test] +fn attachment_path_is_collection_scoped() { + assert_eq!( + classify_path("attachments/prod/a1b2c3d4e5f6a1b2/0011223344556677.enc"), + PathClass::Item { collection: "prod".to_string() } + ); +} + +#[test] +fn attachment_wrong_segment_count_is_rejected() { + assert_eq!( + classify_path("attachments/prod/onlytwo.enc"), + PathClass::Rejected("attachments path must be attachments///.enc".to_string()) + ); +} + +#[test] +fn attachment_empty_or_dotted_slug_is_rejected() { + assert!(matches!(classify_path("attachments//item/att.enc"), PathClass::Rejected(_))); + assert!(matches!(classify_path("attachments/../item/att.enc"), PathClass::Rejected(_))); +} + +#[test] +fn attachments_prefix_alone_is_rejected_not_unrestricted() { + // `attachments/` with no slug/item/att segments must be Rejected, NOT fall + // through to Unrestricted — that fall-through was the authz gap this closes. + assert!(matches!(classify_path("attachments/"), PathClass::Rejected(_))); +} + +#[test] +fn attachment_att_id_segment_may_contain_dots() { + // The `.`-free guard applies to the slug (segment[0]) ONLY; the att-id segment + // legitimately carries `.enc` and is unharmed by additional dots — proving the + // guard is not a blanket "reject any dotted segment". + assert_eq!( + classify_path("attachments/eng/a1b2c3d4e5f6a1b2/00112233.aux.enc"), + PathClass::Item { collection: "eng".to_string() } + ); +} diff --git a/docs/SECURITY.md b/docs/SECURITY.md index c9b04b4..dca0c52 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -111,10 +111,11 @@ before they land. rejected outright. 2. **Path-level write authorisation** — each modified path is classified by - `classify_path` (`crates/relicario-server/src/lib.rs:19`) into - `ProtectedJson` (owner/admin write only), `CollectionItem` (the - `items//…` prefix; write allowed only if the slug appears in the - signer's `collections` grant array), or `Unrestricted`. The write is + `classify_path` (`crates/relicario-server/src/lib.rs:20`) into + `Protected` (owner/admin write only), `Item { collection }` (the + `items//…` or `attachments//…` prefix; write allowed only if + the slug appears in the signer's `collections` grant array), or + `Unrestricted`. The write is authorised if and only if the signer's role and grants satisfy the classification. Item blobs are authorised by the leading path segment alone — the ciphertext is never decrypted by the hook. @@ -132,6 +133,21 @@ before they land. Merge commits are rejected. A genesis commit (no parents) is allowed only when it is signed by the sole Owner it introduces. +#### Attachment write authorisation (v0.1.1 fix) + +Prior to `relicario-server` v0.1.1, `attachments/…` paths fell through to +`PathClass::Unrestricted` in `classify_path` +(`crates/relicario-server/src/lib.rs:20`). Any member with push access could +write attachment blobs to any collection regardless of their grants. As of +v0.1.1, `attachments///.enc` is classified as +`PathClass::Item { collection: slug }`, bringing attachment writes under the +same grant check already applied to `items//.enc` blobs. + +**Deploying this fix requires rebuilding and redeploying the pre-receive hook +on the server.** A server still running a hook built before v0.1.1 continues +to accept attachment pushes from any member; the `Unrestricted` path is only +closed once the updated hook is installed at `/hooks/pre-receive`. + ### Key rotation `relicario org rotate-key` generates a fresh 256-bit org master key,