feat(secrets): group-owned, env-scoped secrets + inherited resolution
Migration 0049 reshapes `secrets` to the same polymorphic-owner contract as `vars` (0048): a secret is owned by an app XOR an ancestor group, carries an `environment_scope`, and the old PK `(app_id, name)` becomes two partial-unique indexes `(owner, environment_scope, name)`. Existing rows backfill to `app_id` + scope `'*'`; the v1 AAD does NOT include the scope, so every current ciphertext keeps decrypting byte-for-byte. `SecretsRepo` is generalised to be owner+scope keyed (`SecretOwner` moves down from `secrets_service` and is re-exported for path stability). The SDK read path now goes through `SecretsRepo::resolve`, which reuses the shared `CHAIN_LEVELS_CTE` to walk app→ancestor-group→root, env-filters, and takes the nearest level (`@E` beating `*` within a level) — returning the winning owner so the value is decrypted under the AAD it was sealed with. Runtime injection stays anchored to `cx.app_id`: an app only ever resolves its own and its ancestors' secrets. All callers updated to owner+scope (`SecretOwner::App(_)`, scope `'*'`): the app-secrets admin API, the SDK service, and the apply email-secret path. The 21 secrets unit tests (incl. the app/group AAD-disjointness and cross-row swap proofs) stay green; the chain-walk was live-verified against Postgres (nearest-wins + env-filter). Admin API for group secrets and the masked-read endpoint land next (Step E). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -22,25 +22,25 @@ use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use picloud_shared::{
|
||||
crypto, validate_secret_name, AppId, GroupId, MasterKey, SdkCallCx, SecretsError,
|
||||
SecretsListPage, SecretsService,
|
||||
crypto, validate_secret_name, MasterKey, SdkCallCx, SecretsError, SecretsListPage,
|
||||
SecretsService,
|
||||
};
|
||||
|
||||
use crate::authz::{self, AuthzRepo, Capability};
|
||||
use crate::secrets_repo::{SecretsRepo, SecretsRepoError, StoredSecret};
|
||||
|
||||
// `SecretOwner` is defined one layer down in `secrets_repo` (it keys both
|
||||
// the storage CRUD and the AAD). Re-exported here so the historical
|
||||
// `secrets_service::SecretOwner` path stays stable.
|
||||
pub use crate::secrets_repo::SecretOwner;
|
||||
|
||||
/// Current AES-GCM envelope version for the per-app secret store.
|
||||
/// `0` = legacy (no AAD); `1` = AAD-bound. New writes always emit v1.
|
||||
pub const SECRET_ENVELOPE_V1: i16 = 1;
|
||||
|
||||
/// Who owns a secret (Phase 3). A secret is owned by exactly one app OR
|
||||
/// one group; the owner is bound into the AES-GCM AAD so a cross-owner
|
||||
/// ciphertext swap fails decryption.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum SecretOwner {
|
||||
App(AppId),
|
||||
Group(GroupId),
|
||||
}
|
||||
/// The env scope under which an app's OWN secrets are stored. App secrets
|
||||
/// are env-agnostic — only group secrets carry a concrete environment.
|
||||
const APP_SECRET_SCOPE: &str = "*";
|
||||
|
||||
/// Audit 2026-06-11 H-D1 — AAD bound into the GCM auth tag so a cross-row
|
||||
/// swap (moving one owner's ciphertext under another's slot) fails
|
||||
@@ -276,10 +276,15 @@ impl SecretsService for SecretsServiceImpl {
|
||||
) -> Result<Option<serde_json::Value>, SecretsError> {
|
||||
validate_secret_name(name)?;
|
||||
self.check_read(cx).await?;
|
||||
let Some(stored) = self.repo.get(cx.app_id, name).await? else {
|
||||
// Inherited resolution: the app's own secret, else the nearest
|
||||
// ancestor group's, env-filtered. `resolve` anchors the walk to
|
||||
// `cx.app_id`, so an app can only ever read its own + its ancestors'
|
||||
// secrets — the cross-app isolation boundary. The winning owner comes
|
||||
// back with the row so we decrypt under the AAD it was sealed with.
|
||||
let Some(resolved) = self.repo.resolve(cx.app_id, name).await? else {
|
||||
return Ok(None);
|
||||
};
|
||||
match open(&self.master_key, SecretOwner::App(cx.app_id), name, &stored) {
|
||||
match open(&self.master_key, resolved.owner, name, &resolved.stored) {
|
||||
Ok(value) => Ok(Some(value)),
|
||||
Err(e) => {
|
||||
// A decrypt failure is operationally significant — surface
|
||||
@@ -303,15 +308,11 @@ impl SecretsService for SecretsServiceImpl {
|
||||
) -> Result<(), SecretsError> {
|
||||
validate_secret_name(name)?;
|
||||
self.check_write(cx).await?;
|
||||
let (ciphertext, nonce, version) = seal(
|
||||
&self.master_key,
|
||||
SecretOwner::App(cx.app_id),
|
||||
name,
|
||||
&value,
|
||||
self.max_value_bytes,
|
||||
)?;
|
||||
let owner = SecretOwner::App(cx.app_id);
|
||||
let (ciphertext, nonce, version) =
|
||||
seal(&self.master_key, owner, name, &value, self.max_value_bytes)?;
|
||||
self.repo
|
||||
.set(cx.app_id, name, &ciphertext, &nonce, version)
|
||||
.set(owner, APP_SECRET_SCOPE, name, &ciphertext, &nonce, version)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -319,7 +320,10 @@ impl SecretsService for SecretsServiceImpl {
|
||||
async fn delete(&self, cx: &SdkCallCx, name: &str) -> Result<bool, SecretsError> {
|
||||
validate_secret_name(name)?;
|
||||
self.check_write(cx).await?;
|
||||
Ok(self.repo.delete(cx.app_id, name).await?)
|
||||
Ok(self
|
||||
.repo
|
||||
.delete(SecretOwner::App(cx.app_id), APP_SECRET_SCOPE, name)
|
||||
.await?)
|
||||
}
|
||||
|
||||
async fn list(
|
||||
@@ -329,7 +333,10 @@ impl SecretsService for SecretsServiceImpl {
|
||||
limit: u32,
|
||||
) -> Result<SecretsListPage, SecretsError> {
|
||||
self.check_read(cx).await?;
|
||||
let page = self.repo.list_names(cx.app_id, cursor, limit).await?;
|
||||
let page = self
|
||||
.repo
|
||||
.list_names(SecretOwner::App(cx.app_id), cursor, limit)
|
||||
.await?;
|
||||
Ok(SecretsListPage {
|
||||
names: page.names,
|
||||
next_cursor: page.next_cursor,
|
||||
@@ -345,44 +352,76 @@ impl SecretsService for SecretsServiceImpl {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::authz::{AuthzError, AuthzRepo};
|
||||
use crate::secrets_repo::{SecretsMetaPage, SecretsNamePage};
|
||||
use crate::secrets_repo::{ResolvedSecret, SecretsMetaPage, SecretsNamePage};
|
||||
use async_trait::async_trait;
|
||||
use picloud_shared::{
|
||||
AdminUserId, AppId, AppRole, ExecutionId, InstanceRole, Principal, RequestId, ScriptId,
|
||||
UserId,
|
||||
AdminUserId, AppId, AppRole, ExecutionId, GroupId, InstanceRole, Principal, RequestId,
|
||||
ScriptId, UserId,
|
||||
};
|
||||
use std::collections::BTreeMap;
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
/// In-memory backing keyed by `(owner_key, env_scope, name)`. The owner
|
||||
/// key is `"app:{uuid}"` / `"group:{uuid}"` so app and group rows never
|
||||
/// collide. These unit tests exercise the app surface (scope `*`); the
|
||||
/// chain-walk `resolve` is journey-tested against real Postgres, so here
|
||||
/// it degrades to a plain app-own `*` lookup.
|
||||
#[derive(Default)]
|
||||
struct InMemorySecretsRepo {
|
||||
data: Mutex<BTreeMap<(AppId, String), StoredSecret>>,
|
||||
data: Mutex<BTreeMap<(String, String, String), StoredSecret>>,
|
||||
}
|
||||
|
||||
fn owner_key(owner: SecretOwner) -> String {
|
||||
match owner {
|
||||
SecretOwner::App(a) => format!("app:{a}"),
|
||||
SecretOwner::Group(g) => format!("group:{g}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl SecretsRepo for InMemorySecretsRepo {
|
||||
async fn get(
|
||||
async fn resolve(
|
||||
&self,
|
||||
app_id: AppId,
|
||||
name: &str,
|
||||
) -> Result<Option<ResolvedSecret>, SecretsRepoError> {
|
||||
let owner = SecretOwner::App(app_id);
|
||||
Ok(self
|
||||
.data
|
||||
.lock()
|
||||
.await
|
||||
.get(&(
|
||||
owner_key(owner),
|
||||
APP_SECRET_SCOPE.to_string(),
|
||||
name.to_string(),
|
||||
))
|
||||
.cloned()
|
||||
.map(|stored| ResolvedSecret { owner, stored }))
|
||||
}
|
||||
async fn get(
|
||||
&self,
|
||||
owner: SecretOwner,
|
||||
env_scope: &str,
|
||||
name: &str,
|
||||
) -> Result<Option<StoredSecret>, SecretsRepoError> {
|
||||
Ok(self
|
||||
.data
|
||||
.lock()
|
||||
.await
|
||||
.get(&(app_id, name.to_string()))
|
||||
.get(&(owner_key(owner), env_scope.to_string(), name.to_string()))
|
||||
.cloned())
|
||||
}
|
||||
async fn set(
|
||||
&self,
|
||||
app_id: AppId,
|
||||
owner: SecretOwner,
|
||||
env_scope: &str,
|
||||
name: &str,
|
||||
encrypted_value: &[u8],
|
||||
nonce: &[u8],
|
||||
version: i16,
|
||||
) -> Result<(), SecretsRepoError> {
|
||||
self.data.lock().await.insert(
|
||||
(app_id, name.to_string()),
|
||||
(owner_key(owner), env_scope.to_string(), name.to_string()),
|
||||
StoredSecret {
|
||||
encrypted_value: encrypted_value.to_vec(),
|
||||
nonce: nonce.to_vec(),
|
||||
@@ -391,29 +430,36 @@ mod tests {
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
async fn delete(&self, app_id: AppId, name: &str) -> Result<bool, SecretsRepoError> {
|
||||
async fn delete(
|
||||
&self,
|
||||
owner: SecretOwner,
|
||||
env_scope: &str,
|
||||
name: &str,
|
||||
) -> Result<bool, SecretsRepoError> {
|
||||
Ok(self
|
||||
.data
|
||||
.lock()
|
||||
.await
|
||||
.remove(&(app_id, name.to_string()))
|
||||
.remove(&(owner_key(owner), env_scope.to_string(), name.to_string()))
|
||||
.is_some())
|
||||
}
|
||||
async fn list_names(
|
||||
&self,
|
||||
app_id: AppId,
|
||||
owner: SecretOwner,
|
||||
cursor: Option<&str>,
|
||||
limit: u32,
|
||||
) -> Result<SecretsNamePage, SecretsRepoError> {
|
||||
let data = self.data.lock().await;
|
||||
let ok = owner_key(owner);
|
||||
let last = cursor.map(std::string::ToString::to_string);
|
||||
let mut names: Vec<String> = data
|
||||
.iter()
|
||||
.filter(|((a, _), _)| *a == app_id)
|
||||
.map(|((_, n), _)| n.clone())
|
||||
.filter(|((o, _, _), _)| *o == ok)
|
||||
.map(|((_, _, n), _)| n.clone())
|
||||
.filter(|n| last.as_ref().is_none_or(|l| n > l))
|
||||
.collect();
|
||||
names.sort();
|
||||
names.dedup();
|
||||
let take = (limit as usize).max(1);
|
||||
let next_cursor = if names.len() > take {
|
||||
names.truncate(take);
|
||||
@@ -425,7 +471,7 @@ mod tests {
|
||||
}
|
||||
async fn list_meta(
|
||||
&self,
|
||||
_app_id: AppId,
|
||||
_owner: SecretOwner,
|
||||
_cursor: Option<&str>,
|
||||
_limit: u32,
|
||||
) -> Result<SecretsMetaPage, SecretsRepoError> {
|
||||
@@ -652,7 +698,11 @@ mod tests {
|
||||
repo.data
|
||||
.lock()
|
||||
.await
|
||||
.get_mut(&(app, "k".to_string()))
|
||||
.get_mut(&(
|
||||
owner_key(SecretOwner::App(app)),
|
||||
"*".to_string(),
|
||||
"k".to_string(),
|
||||
))
|
||||
.unwrap()
|
||||
.encrypted_value[0] ^= 0xff;
|
||||
let err = s.get(&anon_cx(app), "k").await.unwrap_err();
|
||||
@@ -683,10 +733,21 @@ mod tests {
|
||||
.data
|
||||
.lock()
|
||||
.await
|
||||
.get(&(a, "k".to_string()))
|
||||
.get(&(
|
||||
owner_key(SecretOwner::App(a)),
|
||||
"*".to_string(),
|
||||
"k".to_string(),
|
||||
))
|
||||
.cloned()
|
||||
.unwrap();
|
||||
repo.data.lock().await.insert((b, "k".to_string()), stolen);
|
||||
repo.data.lock().await.insert(
|
||||
(
|
||||
owner_key(SecretOwner::App(b)),
|
||||
"*".to_string(),
|
||||
"k".to_string(),
|
||||
),
|
||||
stolen,
|
||||
);
|
||||
let err = s.get(&anon_cx(b), "k").await.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, SecretsError::Corrupted),
|
||||
@@ -761,13 +822,21 @@ mod tests {
|
||||
.data
|
||||
.lock()
|
||||
.await
|
||||
.get(&(a, "real".to_string()))
|
||||
.get(&(
|
||||
owner_key(SecretOwner::App(a)),
|
||||
"*".to_string(),
|
||||
"real".to_string(),
|
||||
))
|
||||
.cloned()
|
||||
.unwrap();
|
||||
repo.data
|
||||
.lock()
|
||||
.await
|
||||
.insert((a, "renamed".to_string()), stolen);
|
||||
repo.data.lock().await.insert(
|
||||
(
|
||||
owner_key(SecretOwner::App(a)),
|
||||
"*".to_string(),
|
||||
"renamed".to_string(),
|
||||
),
|
||||
stolen,
|
||||
);
|
||||
let err = s.get(&anon_cx(a), "renamed").await.unwrap_err();
|
||||
assert!(matches!(err, SecretsError::Corrupted));
|
||||
}
|
||||
@@ -792,7 +861,11 @@ mod tests {
|
||||
)
|
||||
.unwrap();
|
||||
repo.data.lock().await.insert(
|
||||
(app, "old".to_string()),
|
||||
(
|
||||
owner_key(SecretOwner::App(app)),
|
||||
"*".to_string(),
|
||||
"old".to_string(),
|
||||
),
|
||||
StoredSecret {
|
||||
encrypted_value: ct,
|
||||
nonce: nonce.to_vec(),
|
||||
|
||||
Reference in New Issue
Block a user