From e6b479238984460bab374015d940f968deed5493 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 24 Jun 2026 21:48:33 +0200 Subject: [PATCH] feat(secrets): group-owned, env-scoped secrets + inherited resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../migrations/0049_group_secrets.sql | 51 ++++ crates/manager-core/src/apply_service.rs | 8 +- crates/manager-core/src/lib.rs | 2 +- crates/manager-core/src/secrets_api.rs | 29 +- crates/manager-core/src/secrets_repo.rs | 266 +++++++++++++----- crates/manager-core/src/secrets_service.rs | 163 ++++++++--- 6 files changed, 402 insertions(+), 117 deletions(-) create mode 100644 crates/manager-core/migrations/0049_group_secrets.sql diff --git a/crates/manager-core/migrations/0049_group_secrets.sql b/crates/manager-core/migrations/0049_group_secrets.sql new file mode 100644 index 0000000..b1174c5 --- /dev/null +++ b/crates/manager-core/migrations/0049_group_secrets.sql @@ -0,0 +1,51 @@ +-- Phase 3 (v1.1.9): group-owned, environment-scoped secrets. +-- +-- Until now `secrets` was strictly per-app: PK `(app_id, name)`, one +-- envelope per app. Phase 3 makes secrets inheritable down the group tree +-- (blueprint §11 bullet 3 / docs/design §3), exactly like `vars` (0048): +-- a secret may be owned by an app OR an ancestor group, and a descendant +-- app resolves the nearest one, environment-filtered. +-- +-- Reshape (mirrors `vars`): +-- * `group_id` — nullable FK→groups, CASCADE (a deleted group drops its +-- secrets, same as its vars). +-- * `app_id` — made nullable (was NOT NULL); the exactly-one CHECK now +-- enforces "owned by an app XOR a group". +-- * `environment_scope` — `'*'` (env-agnostic) or a concrete env name; +-- the resolver filters on it. Existing rows backfill to `'*'`, so every +-- current app secret stays env-agnostic and resolves unchanged. +-- * PK `(app_id, name)` → two PARTIAL unique indexes, one per owner, both +-- keyed `(owner, environment_scope, name)`. +-- +-- CRYPTO INVARIANT (audit 2026-06-11 H-D1): the v1 AAD is +-- `secret:{app_id}:{name}` for app secrets and `secret:group:{group_id}:{name}` +-- for group secrets — it does NOT include `environment_scope`. Adding the +-- column therefore leaves every existing ciphertext decryptable byte-for-byte; +-- the app-owner AAD is unchanged and the group namespace is disjoint. + +ALTER TABLE secrets + ADD COLUMN group_id UUID REFERENCES groups(id) ON DELETE CASCADE, + ADD COLUMN environment_scope TEXT NOT NULL DEFAULT '*'; + +-- Drop the old composite PK first: `app_id` cannot lose NOT NULL while it +-- is a primary-key column. +ALTER TABLE secrets + DROP CONSTRAINT secrets_pkey; + +ALTER TABLE secrets + ALTER COLUMN app_id DROP NOT NULL; + +ALTER TABLE secrets + ADD CONSTRAINT secrets_owner_exactly_one + CHECK ((group_id IS NULL) <> (app_id IS NULL)); + +-- One secret per (owner, env, name). Partial so each owner column only +-- constrains its own rows; the resolver and every upsert restate the +-- predicate as the ON CONFLICT arbiter. +CREATE UNIQUE INDEX secrets_app_uidx + ON secrets (app_id, environment_scope, name) WHERE app_id IS NOT NULL; +CREATE UNIQUE INDEX secrets_group_uidx + ON secrets (group_id, environment_scope, name) WHERE group_id IS NOT NULL; + +-- Owner lookup index for the group side (the app side keeps idx_secrets_app). +CREATE INDEX idx_secrets_group ON secrets (group_id) WHERE group_id IS NOT NULL; diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index 8ae27f6..e899d0e 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -818,7 +818,7 @@ impl ApplyService { ) -> Result<(Vec, Vec), ApplyError> { let stored = self .secrets - .get(app_id, name) + .get(crate::secrets_service::SecretOwner::App(app_id), "*", name) .await .map_err(|e| ApplyError::Backend(e.to_string()))? .ok_or_else(|| { @@ -1018,7 +1018,11 @@ impl ApplyService { loop { let page = self .secrets - .list_names(app_id, cursor.as_deref(), 200) + .list_names( + crate::secrets_service::SecretOwner::App(app_id), + cursor.as_deref(), + 200, + ) .await .map_err(|e| ApplyError::Backend(e.to_string()))?; names.extend(page.names); diff --git a/crates/manager-core/src/lib.rs b/crates/manager-core/src/lib.rs index d0370c0..ff2daf6 100644 --- a/crates/manager-core/src/lib.rs +++ b/crates/manager-core/src/lib.rs @@ -204,7 +204,7 @@ pub use route_repo::{NewRoute, PostgresRouteRepository, RouteRepository}; pub use sandbox::{CeilingError, SandboxCeiling}; pub use secrets_api::{secrets_router, SecretsApiError, SecretsState}; pub use secrets_repo::{ - PostgresSecretsRepo, SecretMeta, SecretsMetaPage, SecretsNamePage, SecretsRepo, + PostgresSecretsRepo, ResolvedSecret, SecretMeta, SecretsMetaPage, SecretsNamePage, SecretsRepo, SecretsRepoError, StoredSecret, }; pub use secrets_service::{ diff --git a/crates/manager-core/src/secrets_api.rs b/crates/manager-core/src/secrets_api.rs index df910f7..97098ae 100644 --- a/crates/manager-core/src/secrets_api.rs +++ b/crates/manager-core/src/secrets_api.rs @@ -25,9 +25,12 @@ use serde_json::json; use crate::app_repo::AppRepository; use crate::authz::{require, AuthzDenied, AuthzError, AuthzRepo, Capability}; -use crate::secrets_repo::{SecretsRepo, SecretsRepoError}; +use crate::secrets_repo::{SecretOwner, SecretsRepo, SecretsRepoError}; use crate::secrets_service::seal; +/// App secrets are env-agnostic; only group secrets carry a concrete scope. +const APP_SECRET_SCOPE: &str = "*"; + #[derive(Clone)] pub struct SecretsState { pub repo: Arc, @@ -82,7 +85,11 @@ async fn list_secrets( .await?; let page = s .repo - .list_meta(app_id, q.cursor.as_deref(), q.limit.unwrap_or(0)) + .list_meta( + SecretOwner::App(app_id), + q.cursor.as_deref(), + q.limit.unwrap_or(0), + ) .await?; Ok(Json(ListSecretsResponse { secrets: page @@ -121,15 +128,23 @@ async fn set_secret( validate_secret_name(&input.name)?; // Audit 2026-06-11 H-D1 — v1 envelope with AAD bound to // (app_id, name). Same path as the SDK secrets::set. + let owner = SecretOwner::App(app_id); let (ciphertext, nonce, version) = seal( &s.master_key, - crate::secrets_service::SecretOwner::App(app_id), + owner, &input.name, &input.value, s.max_value_bytes, )?; s.repo - .set(app_id, &input.name, &ciphertext, &nonce, version) + .set( + owner, + APP_SECRET_SCOPE, + &input.name, + &ciphertext, + &nonce, + version, + ) .await?; Ok(StatusCode::NO_CONTENT) } @@ -146,7 +161,11 @@ async fn delete_secret( Capability::AppSecretsWrite(app_id), ) .await?; - if !s.repo.delete(app_id, &name).await? { + if !s + .repo + .delete(SecretOwner::App(app_id), APP_SECRET_SCOPE, &name) + .await? + { return Err(SecretsApiError::NotFound); } Ok(StatusCode::NO_CONTENT) diff --git a/crates/manager-core/src/secrets_repo.rs b/crates/manager-core/src/secrets_repo.rs index 9ebc3c8..5343665 100644 --- a/crates/manager-core/src/secrets_repo.rs +++ b/crates/manager-core/src/secrets_repo.rs @@ -2,13 +2,23 @@ //! opaque ciphertext + nonce blobs in and out. Encryption, JSON //! encoding, authorization, name validation, and the value-size cap all //! live one layer up in `SecretsServiceImpl` / `secrets_api`. +//! +//! Phase 3 made secrets polymorphic-owner + env-scoped (migration +//! `0049_group_secrets.sql`): a secret is owned by exactly one app OR one +//! ancestor group, and a descendant app resolves the nearest one, +//! environment-filtered (mirroring `vars` / `config_resolver`). Writes are +//! owner-keyed via [`SecretOwner`]; the SDK read path goes through +//! [`SecretsRepo::resolve`], which walks the app→group→root chain. use async_trait::async_trait; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use base64::Engine as _; use chrono::{DateTime, Utc}; -use picloud_shared::AppId; +use picloud_shared::{AppId, GroupId}; use sqlx::PgPool; +use uuid::Uuid; + +use crate::config_resolver::CHAIN_LEVELS_CTE; #[derive(Debug, thiserror::Error)] pub enum SecretsRepoError { @@ -19,14 +29,22 @@ pub enum SecretsRepoError { InvalidCursor, } +/// 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 (see +/// `secrets_service::secret_aad`) so a cross-owner ciphertext swap fails +/// decryption, and it selects the partial-unique conflict target on write. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SecretOwner { + App(AppId), + Group(GroupId), +} + /// An encrypted secret as it lives on disk: ciphertext (auth tag /// appended) plus the nonce it was sealed with. /// /// Audit 2026-06-11 H-D1: `version` discriminates the envelope layout. /// `0` = legacy AES-GCM with no AAD (pre-2026-06-11 writes); `1` = -/// AES-GCM with AAD bound to `"secret:{app_id}:{name}"`. Migration -/// `0042_secrets_envelope_version.sql` adds the column with a default -/// of `0`, so existing rows keep working. +/// AES-GCM with AAD bound to the owner+name (see `secrets_service`). #[derive(Debug, Clone)] pub struct StoredSecret { pub encrypted_value: Vec, @@ -34,11 +52,21 @@ pub struct StoredSecret { pub version: i16, } +/// The winner of an inherited-secret resolution: the stored ciphertext +/// plus the owner it actually came from (app-own or an ancestor group), +/// which the caller needs to pick the right AAD when decrypting. +#[derive(Debug, Clone)] +pub struct ResolvedSecret { + pub owner: SecretOwner, + pub stored: StoredSecret, +} + /// Admin-surface metadata for one secret. Values are never returned — -/// only the name and the last-modified timestamp. +/// only the name, its environment scope, and the last-modified timestamp. #[derive(Debug, Clone)] pub struct SecretMeta { pub name: String, + pub environment_scope: String, pub updated_at: DateTime, } @@ -60,38 +88,62 @@ pub struct SecretsMetaPage { /// substitute an in-memory backing without Postgres. #[async_trait] pub trait SecretsRepo: Send + Sync { - async fn get( + /// Resolve the effective secret for `app_id` by name: walk the + /// app→ancestor-group chain (depth 0 = the app), env-filter to the + /// app's environment or `*`, and return the nearest winner (with + /// `@E` beating `*` within a level). `None` if no level defines it. + /// This is the runtime injection path — isolation is anchored to + /// `app_id`, so an app only ever sees its own + its ancestors' secrets. + async fn resolve( &self, app_id: AppId, name: &str, + ) -> Result, SecretsRepoError>; + + /// Read one owner's OWN secret at a specific env scope (no inheritance). + /// Backs the group-gated human value-read and the apply email path. + async fn get( + &self, + owner: SecretOwner, + env_scope: &str, + name: &str, ) -> Result, SecretsRepoError>; - /// Upsert (overwrite if present). `version` is the AES-GCM envelope - /// discriminator from [`StoredSecret::version`]. + /// Upsert (overwrite if present) one `(owner, env_scope, name)` row. + /// `version` is the AES-GCM envelope discriminator. async fn set( &self, - app_id: AppId, + owner: SecretOwner, + env_scope: &str, name: &str, encrypted_value: &[u8], nonce: &[u8], version: i16, ) -> Result<(), SecretsRepoError>; - /// Delete; returns whether a row was present. - async fn delete(&self, app_id: AppId, name: &str) -> Result; + /// Delete one `(owner, env_scope, name)` row; returns whether a row + /// was present. + async fn delete( + &self, + owner: SecretOwner, + env_scope: &str, + name: &str, + ) -> Result; - /// Names only — the SDK `list` surface. + /// Distinct names of an owner's OWN secrets (NOT inherited) — the SDK + /// `list` surface. Names are de-duplicated across env scopes. async fn list_names( &self, - app_id: AppId, + owner: SecretOwner, cursor: Option<&str>, limit: u32, ) -> Result; - /// Name + updated_at — the admin `GET` surface. + /// Name + scope + updated_at of an owner's OWN secrets — the admin + /// `GET` surface. async fn list_meta( &self, - app_id: AppId, + owner: SecretOwner, cursor: Option<&str>, limit: u32, ) -> Result; @@ -131,21 +183,77 @@ pub(crate) fn decode_cursor(cursor: &str) -> Result { String::from_utf8(bytes).map_err(|_| SecretsRepoError::InvalidCursor) } +/// `(owner_column, owner_uuid)` for binding an owner into a query. +fn owner_bind(owner: SecretOwner) -> (&'static str, Uuid) { + match owner { + SecretOwner::App(a) => ("app_id", a.into_inner()), + SecretOwner::Group(g) => ("group_id", g.into_inner()), + } +} + #[async_trait] impl SecretsRepo for PostgresSecretsRepo { - async fn get( + async fn resolve( &self, app_id: AppId, name: &str, - ) -> Result, SecretsRepoError> { - let row: Option<(Vec, Vec, i16)> = sqlx::query_as( - "SELECT encrypted_value, nonce, version FROM secrets \ - WHERE app_id = $1 AND name = $2", + ) -> Result, SecretsRepoError> { + // Reuse the shared chain-walk ($1 = app_id), join secrets by name, + // env-filter, and take the nearest level — `@E` beating `*` within a + // level via the secondary sort key. One row out, or none. + let sql = format!( + "{CHAIN_LEVELS_CTE} \ + SELECT CASE WHEN s.app_id IS NOT NULL THEN 'app' ELSE 'group' END AS owner_kind, \ + COALESCE(s.app_id, s.group_id) AS owner_id, \ + s.encrypted_value, s.nonce, s.version \ + FROM chain c \ + JOIN secrets s ON (s.app_id = c.app_owner OR s.group_id = c.group_owner) \ + WHERE s.name = $2 \ + AND (s.environment_scope = '*' OR s.environment_scope = c.app_env) \ + ORDER BY c.depth ASC, (s.environment_scope <> '*') DESC \ + LIMIT 1" + ); + let row: Option<(String, Uuid, Vec, Vec, i16)> = sqlx::query_as(&sql) + .bind(app_id.into_inner()) + .bind(name) + .fetch_optional(&self.pool) + .await?; + Ok( + row.map(|(owner_kind, owner_id, encrypted_value, nonce, version)| { + let owner = if owner_kind == "app" { + SecretOwner::App(AppId::from(owner_id)) + } else { + SecretOwner::Group(GroupId::from(owner_id)) + }; + ResolvedSecret { + owner, + stored: StoredSecret { + encrypted_value, + nonce, + version, + }, + } + }), ) - .bind(app_id.into_inner()) - .bind(name) - .fetch_optional(&self.pool) - .await?; + } + + async fn get( + &self, + owner: SecretOwner, + env_scope: &str, + name: &str, + ) -> Result, SecretsRepoError> { + let (col, id) = owner_bind(owner); + let sql = format!( + "SELECT encrypted_value, nonce, version FROM secrets \ + WHERE {col} = $1 AND environment_scope = $2 AND name = $3" + ); + let row: Option<(Vec, Vec, i16)> = sqlx::query_as(&sql) + .bind(id) + .bind(env_scope) + .bind(name) + .fetch_optional(&self.pool) + .await?; Ok(row.map(|(encrypted_value, nonce, version)| StoredSecret { encrypted_value, nonce, @@ -155,34 +263,54 @@ impl SecretsRepo for PostgresSecretsRepo { async fn set( &self, - app_id: AppId, + owner: SecretOwner, + env_scope: &str, name: &str, encrypted_value: &[u8], nonce: &[u8], version: i16, ) -> Result<(), SecretsRepoError> { - sqlx::query( - "INSERT INTO secrets (app_id, name, encrypted_value, nonce, version) \ - VALUES ($1, $2, $3, $4, $5) \ - ON CONFLICT (app_id, name) DO UPDATE \ + // Owner-kind-specific SQL: write only the owner's nullable column and + // restate the partial-unique predicate as the ON CONFLICT arbiter. + let (col, id) = owner_bind(owner); + let predicate = match owner { + SecretOwner::App(_) => "app_id IS NOT NULL", + SecretOwner::Group(_) => "group_id IS NOT NULL", + }; + let sql = format!( + "INSERT INTO secrets ({col}, environment_scope, name, encrypted_value, nonce, version) \ + VALUES ($1, $2, $3, $4, $5, $6) \ + ON CONFLICT ({col}, environment_scope, name) WHERE {predicate} DO UPDATE \ SET encrypted_value = EXCLUDED.encrypted_value, \ nonce = EXCLUDED.nonce, \ version = EXCLUDED.version, \ - updated_at = NOW()", - ) - .bind(app_id.into_inner()) - .bind(name) - .bind(encrypted_value) - .bind(nonce) - .bind(version) - .execute(&self.pool) - .await?; + updated_at = NOW()" + ); + sqlx::query(&sql) + .bind(id) + .bind(env_scope) + .bind(name) + .bind(encrypted_value) + .bind(nonce) + .bind(version) + .execute(&self.pool) + .await?; Ok(()) } - async fn delete(&self, app_id: AppId, name: &str) -> Result { - let res = sqlx::query("DELETE FROM secrets WHERE app_id = $1 AND name = $2") - .bind(app_id.into_inner()) + async fn delete( + &self, + owner: SecretOwner, + env_scope: &str, + name: &str, + ) -> Result { + let (col, id) = owner_bind(owner); + let sql = format!( + "DELETE FROM secrets WHERE {col} = $1 AND environment_scope = $2 AND name = $3" + ); + let res = sqlx::query(&sql) + .bind(id) + .bind(env_scope) .bind(name) .execute(&self.pool) .await?; @@ -191,7 +319,7 @@ impl SecretsRepo for PostgresSecretsRepo { async fn list_names( &self, - app_id: AppId, + owner: SecretOwner, cursor: Option<&str>, limit: u32, ) -> Result { @@ -201,16 +329,20 @@ impl SecretsRepo for PostgresSecretsRepo { None => None, }; let take = i64::from(limit) + 1; - let rows: Vec<(String,)> = sqlx::query_as( - "SELECT name FROM secrets \ - WHERE app_id = $1 AND ($2::text IS NULL OR name > $2) \ - ORDER BY name ASC LIMIT $3", - ) - .bind(app_id.into_inner()) - .bind(last_name.as_deref()) - .bind(take) - .fetch_all(&self.pool) - .await?; + let (col, id) = owner_bind(owner); + // DISTINCT collapses a name that exists at several env scopes into + // one entry — the SDK `list` is a name catalogue, not per-scope. + let sql = format!( + "SELECT DISTINCT name FROM secrets \ + WHERE {col} = $1 AND ($2::text IS NULL OR name > $2) \ + ORDER BY name ASC LIMIT $3" + ); + let rows: Vec<(String,)> = sqlx::query_as(&sql) + .bind(id) + .bind(last_name.as_deref()) + .bind(take) + .fetch_all(&self.pool) + .await?; let mut names: Vec = rows.into_iter().map(|(n,)| n).collect(); let next_cursor = if names.len() > limit as usize { @@ -224,7 +356,7 @@ impl SecretsRepo for PostgresSecretsRepo { async fn list_meta( &self, - app_id: AppId, + owner: SecretOwner, cursor: Option<&str>, limit: u32, ) -> Result { @@ -234,20 +366,26 @@ impl SecretsRepo for PostgresSecretsRepo { None => None, }; let take = i64::from(limit) + 1; - let rows: Vec<(String, DateTime)> = sqlx::query_as( - "SELECT name, updated_at FROM secrets \ - WHERE app_id = $1 AND ($2::text IS NULL OR name > $2) \ - ORDER BY name ASC LIMIT $3", - ) - .bind(app_id.into_inner()) - .bind(last_name.as_deref()) - .bind(take) - .fetch_all(&self.pool) - .await?; + let (col, id) = owner_bind(owner); + let sql = format!( + "SELECT name, environment_scope, updated_at FROM secrets \ + WHERE {col} = $1 AND ($2::text IS NULL OR name > $2) \ + ORDER BY name ASC, environment_scope ASC LIMIT $3" + ); + let rows: Vec<(String, String, DateTime)> = sqlx::query_as(&sql) + .bind(id) + .bind(last_name.as_deref()) + .bind(take) + .fetch_all(&self.pool) + .await?; let mut items: Vec = rows .into_iter() - .map(|(name, updated_at)| SecretMeta { name, updated_at }) + .map(|(name, environment_scope, updated_at)| SecretMeta { + name, + environment_scope, + updated_at, + }) .collect(); let next_cursor = if items.len() > limit as usize { items.truncate(limit as usize); diff --git a/crates/manager-core/src/secrets_service.rs b/crates/manager-core/src/secrets_service.rs index 17ed3a7..670f13a 100644 --- a/crates/manager-core/src/secrets_service.rs +++ b/crates/manager-core/src/secrets_service.rs @@ -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, 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 { 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 { 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>, + data: Mutex>, + } + + 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, 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, 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 { + async fn delete( + &self, + owner: SecretOwner, + env_scope: &str, + name: &str, + ) -> Result { 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 { let data = self.data.lock().await; + let ok = owner_key(owner); let last = cursor.map(std::string::ToString::to_string); let mut names: Vec = 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 { @@ -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(),