From bb68e5e50aaf42a020f7721995c56ac60784298e Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 24 Jun 2026 22:13:00 +0200 Subject: [PATCH] fix(secrets): composite keyset for group-secret admin list pagination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `list_meta` orders by `(name, environment_scope)` — a group secret name can carry several env-scoped rows — but the cursor encoded only `name` and paginated with `name > $cursor`. When a name's scopes straddled a page boundary, the remaining scope rows were silently skipped from the admin listing. Switch to a composite `(name, environment_scope)` keyset (base64url of `name \x1f scope`) and a Postgres row-comparison predicate. App secrets (single `*` scope per name) were unaffected; group secrets with multi-scope names now page completely. Found in final branch review. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/secrets_repo.rs | 42 +++++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/crates/manager-core/src/secrets_repo.rs b/crates/manager-core/src/secrets_repo.rs index 5343665..f955e64 100644 --- a/crates/manager-core/src/secrets_repo.rs +++ b/crates/manager-core/src/secrets_repo.rs @@ -183,6 +183,27 @@ pub(crate) fn decode_cursor(cursor: &str) -> Result { String::from_utf8(bytes).map_err(|_| SecretsRepoError::InvalidCursor) } +/// `list_meta` orders by `(name, environment_scope)` — a name can now have +/// several env-scoped rows (group secrets) — so its keyset cursor must carry +/// BOTH columns, else a name whose scopes straddle a page boundary loses its +/// tail. Encoded as base64url of `name \x1f scope` (US is not a valid env or +/// secret-name char, so it's an unambiguous delimiter). +const CURSOR_SEP: char = '\u{1f}'; + +fn encode_meta_cursor(name: &str, scope: &str) -> String { + URL_SAFE_NO_PAD.encode(format!("{name}{CURSOR_SEP}{scope}").as_bytes()) +} + +fn decode_meta_cursor(cursor: &str) -> Result<(String, String), SecretsRepoError> { + let bytes = URL_SAFE_NO_PAD + .decode(cursor) + .map_err(|_| SecretsRepoError::InvalidCursor)?; + let s = String::from_utf8(bytes).map_err(|_| SecretsRepoError::InvalidCursor)?; + s.split_once(CURSOR_SEP) + .map(|(n, sc)| (n.to_string(), sc.to_string())) + .ok_or(SecretsRepoError::InvalidCursor) +} + /// `(owner_column, owner_uuid)` for binding an owner into a query. fn owner_bind(owner: SecretOwner) -> (&'static str, Uuid) { match owner { @@ -361,20 +382,27 @@ impl SecretsRepo for PostgresSecretsRepo { limit: u32, ) -> Result { let limit = clamp_limit(limit); - let last_name = match cursor { - Some(c) => Some(decode_cursor(c)?), + // Composite keyset on (name, environment_scope) — see encode_meta_cursor. + let last = match cursor { + Some(c) => Some(decode_meta_cursor(c)?), None => None, }; + let (last_name, last_scope) = match &last { + Some((n, sc)) => (Some(n.as_str()), Some(sc.as_str())), + None => (None, None), + }; let take = i64::from(limit) + 1; 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" + WHERE {col} = $1 \ + AND ($2::text IS NULL OR (name, environment_scope) > ($2, $3)) \ + ORDER BY name ASC, environment_scope ASC LIMIT $4" ); let rows: Vec<(String, String, DateTime)> = sqlx::query_as(&sql) .bind(id) - .bind(last_name.as_deref()) + .bind(last_name) + .bind(last_scope) .bind(take) .fetch_all(&self.pool) .await?; @@ -389,7 +417,9 @@ impl SecretsRepo for PostgresSecretsRepo { .collect(); let next_cursor = if items.len() > limit as usize { items.truncate(limit as usize); - items.last().map(|m| encode_cursor(&m.name)) + items + .last() + .map(|m| encode_meta_cursor(&m.name, &m.environment_scope)) } else { None };