From 9091ac6f5c56ac0684a03ed8c3c3e55fbe7dcec6 Mon Sep 17 00:00:00 2001 From: Bruno Date: Tue, 25 Nov 2025 10:04:43 -0300 Subject: [PATCH] Secrets: add basic namespace and name checks to keeper store and secure value store (#114355) --- ...alue_get_latest_version_and_created_at.sql | 4 ++- .../data/secure_value_list_by_lease_token.sql | 3 +- pkg/storage/secret/metadata/keeper_store.go | 12 ++++++++ .../secret/metadata/secure_value_store.go | 30 ++++++++++++++++--- ...ted_at-get latest secure value version.sql | 4 ++- ...ist_by_lease_token-list by lease token.sql | 3 +- ...ted_at-get latest secure value version.sql | 4 ++- ...ist_by_lease_token-list by lease token.sql | 3 +- ...ted_at-get latest secure value version.sql | 4 ++- ...ist_by_lease_token-list by lease token.sql | 3 +- 10 files changed, 58 insertions(+), 12 deletions(-) diff --git a/pkg/storage/secret/metadata/data/secure_value_get_latest_version_and_created_at.sql b/pkg/storage/secret/metadata/data/secure_value_get_latest_version_and_created_at.sql index 1c804c2d031..c7349f27a03 100644 --- a/pkg/storage/secret/metadata/data/secure_value_get_latest_version_and_created_at.sql +++ b/pkg/storage/secret/metadata/data/secure_value_get_latest_version_and_created_at.sql @@ -1,7 +1,9 @@ SELECT {{ .Ident "created" }}, {{ .Ident "version" }}, - {{ .Ident "active" }} + {{ .Ident "active" }}, + {{ .Ident "namespace" }}, + {{ .Ident "name" }} FROM {{ .Ident "secret_secure_value" }} WHERE diff --git a/pkg/storage/secret/metadata/data/secure_value_list_by_lease_token.sql b/pkg/storage/secret/metadata/data/secure_value_list_by_lease_token.sql index 275143a9672..fc62c271df4 100644 --- a/pkg/storage/secret/metadata/data/secure_value_list_by_lease_token.sql +++ b/pkg/storage/secret/metadata/data/secure_value_list_by_lease_token.sql @@ -18,7 +18,8 @@ SELECT {{ .Ident "owner_reference_api_group" }}, {{ .Ident "owner_reference_api_version" }}, {{ .Ident "owner_reference_kind" }}, - {{ .Ident "owner_reference_name" }} + {{ .Ident "owner_reference_name" }}, + {{ .Ident "lease_token" }} FROM {{ .Ident "secret_secure_value" }} WHERE diff --git a/pkg/storage/secret/metadata/keeper_store.go b/pkg/storage/secret/metadata/keeper_store.go index 6036bfe2502..d4516158c81 100644 --- a/pkg/storage/secret/metadata/keeper_store.go +++ b/pkg/storage/secret/metadata/keeper_store.go @@ -201,6 +201,10 @@ func (s *keeperMetadataStorage) read(ctx context.Context, namespace, name string if err := res.Err(); err != nil { return nil, fmt.Errorf("read rows error: %w", err) } + if keeper.Namespace != namespace || keeper.Name != name { + return nil, fmt.Errorf("bug: expected to find keeper namespace=%+v name=%+v but got keeper namespace=%+v name%+v", + namespace, name, keeper.Namespace, keeper.Name) + } return &keeper, nil } @@ -405,6 +409,10 @@ func (s *keeperMetadataStorage) List(ctx context.Context, namespace xkube.Namesp return nil, fmt.Errorf("error reading keeper row: %w", err) } + if row.Namespace != namespace.String() { + return nil, fmt.Errorf("bug: expected to list keepers for namespace %+v but got one from namespace %+v", namespace, row.Namespace) + } + keeper, err := row.toKubernetes() if err != nil { return nil, fmt.Errorf("failed to convert to kubernetes object: %w", err) @@ -706,6 +714,10 @@ func (s *keeperMetadataStorage) GetActiveKeeper(ctx context.Context, namespace s return keeper, fmt.Errorf("converting from keeperDB to kubernetes struct: %w", err) } + if keeperDB.Namespace != namespace { + return nil, fmt.Errorf("bug: expected to find keeper to namespace %+v but got one for namespace %+v", namespace, keeperDB.Namespace) + } + return keeper, nil } diff --git a/pkg/storage/secret/metadata/secure_value_store.go b/pkg/storage/secret/metadata/secure_value_store.go index d3a137ea611..1fc3a788cf2 100644 --- a/pkg/storage/secret/metadata/secure_value_store.go +++ b/pkg/storage/secret/metadata/secure_value_store.go @@ -199,14 +199,21 @@ func (s *secureValueMetadataStorage) getLatestVersionAndCreatedAt(ctx context.Co } var ( - createdAt int64 - version int64 - active bool + createdAt int64 + version int64 + active bool + namespaceFromDB string + nameFromDB string ) - if err := rows.Scan(&createdAt, &version, &active); err != nil { + if err := rows.Scan(&createdAt, &version, &active, &namespaceFromDB, &nameFromDB); err != nil { return versionAndCreatedAt{}, fmt.Errorf("scanning version from returned rows: %w", err) } + if namespaceFromDB != namespace.String() || nameFromDB != name { + return versionAndCreatedAt{}, fmt.Errorf("bug: expected to find latest version for namespace=%+v name=%+v but got version for namespace=%+v name=%+v", + namespace, name, namespaceFromDB, nameFromDB) + } + if !active { createdAt = 0 } @@ -255,6 +262,11 @@ func (s *secureValueMetadataStorage) readActiveVersion(ctx context.Context, name if err := res.Err(); err != nil { return secureValueDB{}, fmt.Errorf("read rows error: %w", err) } + + if secureValue.Namespace != namespace.String() || secureValue.Name != name { + return secureValueDB{}, fmt.Errorf("bug: expected to read secure value %+v from namespace %+v, but got a different row", name, namespace) + } + return secureValue, nil } @@ -364,6 +376,10 @@ func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.N return nil, fmt.Errorf("bug: read an inactive version: row=%+v", row) } + if row.Namespace != namespace.String() { + return nil, fmt.Errorf("bug: expected to list secure values from namespace %+v but got one from namespace %+v", namespace.String(), row.Namespace) + } + secureValue, err := row.toKubernetes() if err != nil { return nil, fmt.Errorf("convert to kubernetes object: %w", err) @@ -644,6 +660,7 @@ func (s *secureValueMetadataStorage) listByLeaseToken(ctx context.Context, lease secureValues := make([]secretv1beta1.SecureValue, 0) for rows.Next() { row := secureValueDB{} + var leaseTokenDB string err = rows.Scan(&row.GUID, &row.Name, &row.Namespace, &row.Annotations, @@ -653,12 +670,17 @@ func (s *secureValueMetadataStorage) listByLeaseToken(ctx context.Context, lease &row.Description, &row.Keeper, &row.Decrypters, &row.Ref, &row.ExternalID, &row.Version, &row.Active, &row.OwnerReferenceAPIGroup, &row.OwnerReferenceAPIVersion, &row.OwnerReferenceKind, &row.OwnerReferenceName, + &leaseTokenDB, ) if err != nil { return nil, fmt.Errorf("error reading secure value row: %w", err) } + if leaseTokenDB != leaseToken { + return nil, fmt.Errorf("bug: expected to list secure values with lease token %+v but got a secure value with another lease token %+v", leaseToken, leaseToken) + } + secureValue, err := row.toKubernetes() if err != nil { return nil, fmt.Errorf("convert to kubernetes object: %w", err) diff --git a/pkg/storage/secret/metadata/testdata/mysql--secure_value_get_latest_version_and_created_at-get latest secure value version.sql b/pkg/storage/secret/metadata/testdata/mysql--secure_value_get_latest_version_and_created_at-get latest secure value version.sql index 661c6474221..2e88ee079c2 100755 --- a/pkg/storage/secret/metadata/testdata/mysql--secure_value_get_latest_version_and_created_at-get latest secure value version.sql +++ b/pkg/storage/secret/metadata/testdata/mysql--secure_value_get_latest_version_and_created_at-get latest secure value version.sql @@ -1,7 +1,9 @@ SELECT `created`, `version`, - `active` + `active`, + `namespace`, + `name` FROM `secret_secure_value` WHERE diff --git a/pkg/storage/secret/metadata/testdata/mysql--secure_value_list_by_lease_token-list by lease token.sql b/pkg/storage/secret/metadata/testdata/mysql--secure_value_list_by_lease_token-list by lease token.sql index 07534407789..0485a5c94c1 100755 --- a/pkg/storage/secret/metadata/testdata/mysql--secure_value_list_by_lease_token-list by lease token.sql +++ b/pkg/storage/secret/metadata/testdata/mysql--secure_value_list_by_lease_token-list by lease token.sql @@ -18,7 +18,8 @@ SELECT `owner_reference_api_group`, `owner_reference_api_version`, `owner_reference_kind`, - `owner_reference_name` + `owner_reference_name`, + `lease_token` FROM `secret_secure_value` WHERE diff --git a/pkg/storage/secret/metadata/testdata/postgres--secure_value_get_latest_version_and_created_at-get latest secure value version.sql b/pkg/storage/secret/metadata/testdata/postgres--secure_value_get_latest_version_and_created_at-get latest secure value version.sql index e1c3834ecbb..a9f21aafc0a 100755 --- a/pkg/storage/secret/metadata/testdata/postgres--secure_value_get_latest_version_and_created_at-get latest secure value version.sql +++ b/pkg/storage/secret/metadata/testdata/postgres--secure_value_get_latest_version_and_created_at-get latest secure value version.sql @@ -1,7 +1,9 @@ SELECT "created", "version", - "active" + "active", + "namespace", + "name" FROM "secret_secure_value" WHERE diff --git a/pkg/storage/secret/metadata/testdata/postgres--secure_value_list_by_lease_token-list by lease token.sql b/pkg/storage/secret/metadata/testdata/postgres--secure_value_list_by_lease_token-list by lease token.sql index 55077bff201..0829c0a7ceb 100755 --- a/pkg/storage/secret/metadata/testdata/postgres--secure_value_list_by_lease_token-list by lease token.sql +++ b/pkg/storage/secret/metadata/testdata/postgres--secure_value_list_by_lease_token-list by lease token.sql @@ -18,7 +18,8 @@ SELECT "owner_reference_api_group", "owner_reference_api_version", "owner_reference_kind", - "owner_reference_name" + "owner_reference_name", + "lease_token" FROM "secret_secure_value" WHERE diff --git a/pkg/storage/secret/metadata/testdata/sqlite--secure_value_get_latest_version_and_created_at-get latest secure value version.sql b/pkg/storage/secret/metadata/testdata/sqlite--secure_value_get_latest_version_and_created_at-get latest secure value version.sql index e1c3834ecbb..a9f21aafc0a 100755 --- a/pkg/storage/secret/metadata/testdata/sqlite--secure_value_get_latest_version_and_created_at-get latest secure value version.sql +++ b/pkg/storage/secret/metadata/testdata/sqlite--secure_value_get_latest_version_and_created_at-get latest secure value version.sql @@ -1,7 +1,9 @@ SELECT "created", "version", - "active" + "active", + "namespace", + "name" FROM "secret_secure_value" WHERE diff --git a/pkg/storage/secret/metadata/testdata/sqlite--secure_value_list_by_lease_token-list by lease token.sql b/pkg/storage/secret/metadata/testdata/sqlite--secure_value_list_by_lease_token-list by lease token.sql index 55077bff201..0829c0a7ceb 100755 --- a/pkg/storage/secret/metadata/testdata/sqlite--secure_value_list_by_lease_token-list by lease token.sql +++ b/pkg/storage/secret/metadata/testdata/sqlite--secure_value_list_by_lease_token-list by lease token.sql @@ -18,7 +18,8 @@ SELECT "owner_reference_api_group", "owner_reference_api_version", "owner_reference_kind", - "owner_reference_name" + "owner_reference_name", + "lease_token" FROM "secret_secure_value" WHERE