feat(kvstore): store token branch prefixes and bump the schema to 2
An older build would read a branch-limited token as unrestricted, so the schema moves to 2: an older binary refuses the data (ErrSchemaTooNew). A version 1 store is upgraded when opened.
- commit
- 616a749b5ce45af09787e1eee753c40299ffd25c
- author
- brice · · 2026-10-12 14:54 UTC
- committer
- brice · · 2026-10-12 15:01 UTC
- parent
- 9dac236
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +134 -15
M gen/ermite/storage/repo.pb.go +16 -5
| @@ -226,9 +226,12 @@ type Token struct { | |||
| 226 | 226 | ExpiresAt *timestamppb.Timestamp `protobuf:"bytes,6,opt,name=expires_at,json=expiresAt,proto3" json:"expires_at,omitempty"` | |
| 227 | 227 | CreatedAt *timestamppb.Timestamp `protobuf:"bytes,7,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` | |
| 228 | 228 | // Unset until the token is first used. | |
| 229 | - | LastUsedAt *timestamppb.Timestamp `protobuf:"bytes,8,opt,name=last_used_at,json=lastUsedAt,proto3" json:"last_used_at,omitempty"` | |
| 230 | - | unknownFields protoimpl.UnknownFields | |
| 231 | - | sizeCache protoimpl.SizeCache | |
| 229 | + | LastUsedAt *timestamppb.Timestamp `protobuf:"bytes,8,opt,name=last_used_at,json=lastUsedAt,proto3" json:"last_used_at,omitempty"` | |
| 230 | + | // Branch prefixes, each ending with "/", that the token may write; empty | |
| 231 | + | // means every branch. Added in storage schema 2. | |
| 232 | + | BranchPrefixes []string `protobuf:"bytes,9,rep,name=branch_prefixes,json=branchPrefixes,proto3" json:"branch_prefixes,omitempty"` | |
| 233 | + | unknownFields protoimpl.UnknownFields | |
| 234 | + | sizeCache protoimpl.SizeCache | |
| 232 | 235 | } | |
| 233 | 236 | ||
| 234 | 237 | func (x *Token) Reset() { | |
| @@ -317,6 +320,13 @@ func (x *Token) GetLastUsedAt() *timestamppb.Timestamp { | |||
| 317 | 320 | return nil | |
| 318 | 321 | } | |
| 319 | 322 | ||
| 323 | + | func (x *Token) GetBranchPrefixes() []string { | |
| 324 | + | if x != nil { | |
| 325 | + | return x.BranchPrefixes | |
| 326 | + | } | |
| 327 | + | return nil | |
| 328 | + | } | |
| 329 | + | ||
| 320 | 330 | var File_ermite_storage_repo_proto protoreflect.FileDescriptor | |
| 321 | 331 | ||
| 322 | 332 | const file_ermite_storage_repo_proto_rawDesc = "" + | |
| @@ -333,7 +343,7 @@ const file_ermite_storage_repo_proto_rawDesc = "" + | |||
| 333 | 343 | "\tpushed_at\x18\x05 \x01(\v2\x1a.google.protobuf.TimestampR\bpushedAt\x128\n" + | |
| 334 | 344 | "\bsettings\x18\x06 \x01(\v2\x1c.ermite.storage.RepoSettingsR\bsettings\")\n" + | |
| 335 | 345 | "\rSchemaVersion\x12\x18\n" + | |
| 336 | - | "\aversion\x18\x01 \x01(\rR\aversion\"\xa3\x02\n" + | |
| 346 | + | "\aversion\x18\x01 \x01(\rR\aversion\"\xcc\x02\n" + | |
| 337 | 347 | "\x05Token\x12\x0e\n" + | |
| 338 | 348 | "\x02id\x18\x01 \x01(\tR\x02id\x12\x14\n" + | |
| 339 | 349 | "\x05label\x18\x02 \x01(\tR\x05label\x12\x12\n" + | |
| @@ -345,7 +355,8 @@ const file_ermite_storage_repo_proto_rawDesc = "" + | |||
| 345 | 355 | "\n" + | |
| 346 | 356 | "created_at\x18\a \x01(\v2\x1a.google.protobuf.TimestampR\tcreatedAt\x12<\n" + | |
| 347 | 357 | "\flast_used_at\x18\b \x01(\v2\x1a.google.protobuf.TimestampR\n" + | |
| 348 | - | "lastUsedAt*W\n" + | |
| 358 | + | "lastUsedAt\x12'\n" + | |
| 359 | + | "\x0fbranch_prefixes\x18\t \x03(\tR\x0ebranchPrefixes*W\n" + | |
| 349 | 360 | "\n" + | |
| 350 | 361 | "Visibility\x12\x1a\n" + | |
| 351 | 362 | "\x16VISIBILITY_UNSPECIFIED\x10\x00\x12\x16\n" + | |
| @@ -226,9 +226,12 @@ type Token struct { | |||||
| 226 | ExpiresAt *timestamppb.Timestamp `protobuf:"bytes,6,opt,name=expires_at,json=expiresAt,proto3" json:"expires_at,omitempty"` | 226 | ExpiresAt *timestamppb.Timestamp `protobuf:"bytes,6,opt,name=expires_at,json=expiresAt,proto3" json:"expires_at,omitempty"` | ||
| 227 | CreatedAt *timestamppb.Timestamp `protobuf:"bytes,7,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` | 227 | CreatedAt *timestamppb.Timestamp `protobuf:"bytes,7,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` | ||
| 228 | // Unset until the token is first used. | 228 | // Unset until the token is first used. | ||
| 229 | - | LastUsedAt *timestamppb.Timestamp `protobuf:"bytes,8,opt,name=last_used_at,json=lastUsedAt,proto3" json:"last_used_at,omitempty"` | 229 | + | LastUsedAt *timestamppb.Timestamp `protobuf:"bytes,8,opt,name=last_used_at,json=lastUsedAt,proto3" json:"last_used_at,omitempty"` |
| 230 | - | unknownFields protoimpl.UnknownFields | 230 | + | // Branch prefixes, each ending with "/", that the token may write; empty |
| 231 | - | sizeCache protoimpl.SizeCache | 231 | + | // means every branch. Added in storage schema 2. |
| 232 | + | BranchPrefixes []string `protobuf:"bytes,9,rep,name=branch_prefixes,json=branchPrefixes,proto3" json:"branch_prefixes,omitempty"` | |||
| 233 | + | unknownFields protoimpl.UnknownFields | |||
| 234 | + | sizeCache protoimpl.SizeCache | |||
| 232 | } | 235 | } | ||
| 233 | 236 | ||||
| 234 | func (x *Token) Reset() { | 237 | func (x *Token) Reset() { | ||
| @@ -317,6 +320,13 @@ func (x *Token) GetLastUsedAt() *timestamppb.Timestamp { | |||||
| 317 | return nil | 320 | return nil | ||
| 318 | } | 321 | } | ||
| 319 | 322 | ||||
| 323 | + | func (x *Token) GetBranchPrefixes() []string { | |||
| 324 | + | if x != nil { | |||
| 325 | + | return x.BranchPrefixes | |||
| 326 | + | } | |||
| 327 | + | return nil | |||
| 328 | + | } | |||
| 329 | + | ||||
| 320 | var File_ermite_storage_repo_proto protoreflect.FileDescriptor | 330 | var File_ermite_storage_repo_proto protoreflect.FileDescriptor | ||
| 321 | 331 | ||||
| 322 | const file_ermite_storage_repo_proto_rawDesc = "" + | 332 | const file_ermite_storage_repo_proto_rawDesc = "" + | ||
| @@ -333,7 +343,7 @@ const file_ermite_storage_repo_proto_rawDesc = "" + | |||||
| 333 | "\tpushed_at\x18\x05 \x01(\v2\x1a.google.protobuf.TimestampR\bpushedAt\x128\n" + | 343 | "\tpushed_at\x18\x05 \x01(\v2\x1a.google.protobuf.TimestampR\bpushedAt\x128\n" + | ||
| 334 | "\bsettings\x18\x06 \x01(\v2\x1c.ermite.storage.RepoSettingsR\bsettings\")\n" + | 344 | "\bsettings\x18\x06 \x01(\v2\x1c.ermite.storage.RepoSettingsR\bsettings\")\n" + | ||
| 335 | "\rSchemaVersion\x12\x18\n" + | 345 | "\rSchemaVersion\x12\x18\n" + | ||
| 336 | - | "\aversion\x18\x01 \x01(\rR\aversion\"\xa3\x02\n" + | 346 | + | "\aversion\x18\x01 \x01(\rR\aversion\"\xcc\x02\n" + |
| 337 | "\x05Token\x12\x0e\n" + | 347 | "\x05Token\x12\x0e\n" + | ||
| 338 | "\x02id\x18\x01 \x01(\tR\x02id\x12\x14\n" + | 348 | "\x02id\x18\x01 \x01(\tR\x02id\x12\x14\n" + | ||
| 339 | "\x05label\x18\x02 \x01(\tR\x05label\x12\x12\n" + | 349 | "\x05label\x18\x02 \x01(\tR\x05label\x12\x12\n" + | ||
| @@ -345,7 +355,8 @@ const file_ermite_storage_repo_proto_rawDesc = "" + | |||||
| 345 | "\n" + | 355 | "\n" + | ||
| 346 | "created_at\x18\a \x01(\v2\x1a.google.protobuf.TimestampR\tcreatedAt\x12<\n" + | 356 | "created_at\x18\a \x01(\v2\x1a.google.protobuf.TimestampR\tcreatedAt\x12<\n" + | ||
| 347 | "\flast_used_at\x18\b \x01(\v2\x1a.google.protobuf.TimestampR\n" + | 357 | "\flast_used_at\x18\b \x01(\v2\x1a.google.protobuf.TimestampR\n" + | ||
| 348 | - | "lastUsedAt*W\n" + | 358 | + | "lastUsedAt\x12'\n" + |
| 359 | + | "\x0fbranch_prefixes\x18\t \x03(\tR\x0ebranchPrefixes*W\n" + | |||
| 349 | "\n" + | 360 | "\n" + | ||
| 350 | "Visibility\x12\x1a\n" + | 361 | "Visibility\x12\x1a\n" + | ||
| 351 | "\x16VISIBILITY_UNSPECIFIED\x10\x00\x12\x16\n" + | 362 | "\x16VISIBILITY_UNSPECIFIED\x10\x00\x12\x16\n" + | ||
M internal/adapters/kvstore/store.go +23 -4
| @@ -17,8 +17,11 @@ import ( | |||
| 17 | 17 | "bnema.dev/ermite/internal/ports" | |
| 18 | 18 | ) | |
| 19 | 19 | ||
| 20 | - | // SchemaVersion is the storage schema this build reads and writes. | |
| 21 | - | const SchemaVersion = 1 | |
| 20 | + | // SchemaVersion is the storage schema this build reads and writes. Version 2 | |
| 21 | + | // adds branch prefixes to tokens: a build that does not know them would treat | |
| 22 | + | // a branch-limited token as unrestricted, so it must refuse the data | |
| 23 | + | // (ErrSchemaTooNew) instead. | |
| 24 | + | const SchemaVersion = 2 | |
| 22 | 25 | ||
| 23 | 26 | const schemaKey = "meta/schema" | |
| 24 | 27 | ||
| @@ -33,8 +36,8 @@ type Store struct { | |||
| 33 | 36 | var _ ports.RepoMetaStore = (*Store)(nil) | |
| 34 | 37 | ||
| 35 | 38 | // Open opens (or creates) the database in dir and checks the storage schema: | |
| 36 | - | // a missing version is written as SchemaVersion, a higher one stops with | |
| 37 | - | // ErrSchemaTooNew. dir must exist. | |
| 39 | + | // a missing version is written as SchemaVersion, a lower one is upgraded to it, | |
| 40 | + | // a higher one stops with ErrSchemaTooNew. dir must exist. | |
| 38 | 41 | func Open(ctx context.Context, dir string) (*Store, error) { | |
| 39 | 42 | db, err := kv.Open(ctx, dir, kv.Options{}) | |
| 40 | 43 | if err != nil { | |
| @@ -107,6 +110,22 @@ func (s *Store) checkSchema(ctx context.Context) error { | |||
| 107 | 110 | case sv.GetVersion() == 0: | |
| 108 | 111 | return fmt.Errorf("decode storage schema: invalid version 0") | |
| 109 | 112 | } | |
| 113 | + | if sv.GetVersion() < SchemaVersion { | |
| 114 | + | // Older data stays readable as it is (the changes are additive); | |
| 115 | + | // the new version is recorded so that older builds refuse it. | |
| 116 | + | var b kv.Batch | |
| 117 | + | b.RequireVersion(key, item.Version) | |
| 118 | + | b.Put(key, mustMarshal(&storagepb.SchemaVersion{Version: SchemaVersion})) | |
| 119 | + | _, err := s.db.Apply(ctx, &b) | |
| 120 | + | if errors.Is(err, kv.ErrConflict) { | |
| 121 | + | continue // changed meanwhile: read it again | |
| 122 | + | } | |
| 123 | + | if err != nil { | |
| 124 | + | return fmt.Errorf("upgrade storage schema: %w", err) | |
| 125 | + | } | |
| 126 | + | log.Info().Uint32("from", sv.GetVersion()).Uint32("schema", SchemaVersion).Msg("storage schema upgraded") | |
| 127 | + | return nil | |
| 128 | + | } | |
| 110 | 129 | log.Info().Uint32("schema", sv.GetVersion()).Msg("storage schema checked") | |
| 111 | 130 | return nil | |
| 112 | 131 | } | |
| @@ -17,8 +17,11 @@ import ( | |||||
| 17 | "bnema.dev/ermite/internal/ports" | 17 | "bnema.dev/ermite/internal/ports" | ||
| 18 | ) | 18 | ) | ||
| 19 | 19 | ||||
| 20 | - | // SchemaVersion is the storage schema this build reads and writes. | 20 | + | // SchemaVersion is the storage schema this build reads and writes. Version 2 |
| 21 | - | const SchemaVersion = 1 | 21 | + | // adds branch prefixes to tokens: a build that does not know them would treat |
| 22 | + | // a branch-limited token as unrestricted, so it must refuse the data | |||
| 23 | + | // (ErrSchemaTooNew) instead. | |||
| 24 | + | const SchemaVersion = 2 | |||
| 22 | 25 | ||||
| 23 | const schemaKey = "meta/schema" | 26 | const schemaKey = "meta/schema" | ||
| 24 | 27 | ||||
| @@ -33,8 +36,8 @@ type Store struct { | |||||
| 33 | var _ ports.RepoMetaStore = (*Store)(nil) | 36 | var _ ports.RepoMetaStore = (*Store)(nil) | ||
| 34 | 37 | ||||
| 35 | // Open opens (or creates) the database in dir and checks the storage schema: | 38 | // Open opens (or creates) the database in dir and checks the storage schema: | ||
| 36 | - | // a missing version is written as SchemaVersion, a higher one stops with | 39 | + | // a missing version is written as SchemaVersion, a lower one is upgraded to it, |
| 37 | - | // ErrSchemaTooNew. dir must exist. | 40 | + | // a higher one stops with ErrSchemaTooNew. dir must exist. |
| 38 | func Open(ctx context.Context, dir string) (*Store, error) { | 41 | func Open(ctx context.Context, dir string) (*Store, error) { | ||
| 39 | db, err := kv.Open(ctx, dir, kv.Options{}) | 42 | db, err := kv.Open(ctx, dir, kv.Options{}) | ||
| 40 | if err != nil { | 43 | if err != nil { | ||
| @@ -107,6 +110,22 @@ func (s *Store) checkSchema(ctx context.Context) error { | |||||
| 107 | case sv.GetVersion() == 0: | 110 | case sv.GetVersion() == 0: | ||
| 108 | return fmt.Errorf("decode storage schema: invalid version 0") | 111 | return fmt.Errorf("decode storage schema: invalid version 0") | ||
| 109 | } | 112 | } | ||
| 113 | + | if sv.GetVersion() < SchemaVersion { | |||
| 114 | + | // Older data stays readable as it is (the changes are additive); | |||
| 115 | + | // the new version is recorded so that older builds refuse it. | |||
| 116 | + | var b kv.Batch | |||
| 117 | + | b.RequireVersion(key, item.Version) | |||
| 118 | + | b.Put(key, mustMarshal(&storagepb.SchemaVersion{Version: SchemaVersion})) | |||
| 119 | + | _, err := s.db.Apply(ctx, &b) | |||
| 120 | + | if errors.Is(err, kv.ErrConflict) { | |||
| 121 | + | continue // changed meanwhile: read it again | |||
| 122 | + | } | |||
| 123 | + | if err != nil { | |||
| 124 | + | return fmt.Errorf("upgrade storage schema: %w", err) | |||
| 125 | + | } | |||
| 126 | + | log.Info().Uint32("from", sv.GetVersion()).Uint32("schema", SchemaVersion).Msg("storage schema upgraded") | |||
| 127 | + | return nil | |||
| 128 | + | } | |||
| 110 | log.Info().Uint32("schema", sv.GetVersion()).Msg("storage schema checked") | 129 | log.Info().Uint32("schema", sv.GetVersion()).Msg("storage schema checked") | ||
| 111 | return nil | 130 | return nil | ||
| 112 | } | 131 | } | ||
M internal/adapters/kvstore/store_test.go +19 -5
| @@ -191,12 +191,13 @@ func readSchema(t *testing.T, dir string) uint32 { | |||
| 191 | 191 | func TestSchema(t *testing.T) { | |
| 192 | 192 | ctx := context.Background() | |
| 193 | 193 | ||
| 194 | - | t.Run("missing is written as 1", func(t *testing.T) { | |
| 194 | + | t.Run("missing is written as the current version", func(t *testing.T) { | |
| 195 | 195 | dir := t.TempDir() | |
| 196 | 196 | s, err := kvstore.Open(ctx, dir) | |
| 197 | 197 | require.NoError(t, err) | |
| 198 | 198 | require.NoError(t, s.Close(ctx)) | |
| 199 | - | assert.Equal(t, uint32(1), readSchema(t, dir)) | |
| 199 | + | assert.Equal(t, uint32(kvstore.SchemaVersion), readSchema(t, dir)) | |
| 200 | + | assert.Equal(t, uint32(2), uint32(kvstore.SchemaVersion), "branch prefixes need schema 2") | |
| 200 | 201 | }) | |
| 201 | 202 | ||
| 202 | 203 | t.Run("current is accepted and kept", func(t *testing.T) { | |
| @@ -207,15 +208,28 @@ func TestSchema(t *testing.T) { | |||
| 207 | 208 | s, err = kvstore.Open(ctx, dir) | |
| 208 | 209 | require.NoError(t, err) | |
| 209 | 210 | require.NoError(t, s.Close(ctx)) | |
| 210 | - | assert.Equal(t, uint32(1), readSchema(t, dir)) | |
| 211 | + | assert.Equal(t, uint32(kvstore.SchemaVersion), readSchema(t, dir)) | |
| 212 | + | }) | |
| 213 | + | ||
| 214 | + | t.Run("version 1 is upgraded to 2, its data kept", func(t *testing.T) { | |
| 215 | + | dir := t.TempDir() | |
| 216 | + | putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 1})) | |
| 217 | + | putRaw(t, dir, "repo/old", mustProto(t, &storagepb.Repo{Name: "old", Visibility: storagepb.Visibility_VISIBILITY_PUBLIC})) | |
| 218 | + | s, err := kvstore.Open(ctx, dir) | |
| 219 | + | require.NoError(t, err) | |
| 220 | + | got, err := s.Get(ctx, name(t, "old")) | |
| 221 | + | require.NoError(t, err) | |
| 222 | + | assert.Equal(t, "old", got.Name.String()) | |
| 223 | + | require.NoError(t, s.Close(ctx)) | |
| 224 | + | assert.Equal(t, uint32(2), readSchema(t, dir)) | |
| 211 | 225 | }) | |
| 212 | 226 | ||
| 213 | 227 | t.Run("newer stops startup and releases the lock", func(t *testing.T) { | |
| 214 | 228 | dir := t.TempDir() | |
| 215 | - | putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 2})) | |
| 229 | + | putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 3})) | |
| 216 | 230 | _, err := kvstore.Open(ctx, dir) | |
| 217 | 231 | require.ErrorIs(t, err, kvstore.ErrSchemaTooNew) | |
| 218 | - | assert.Equal(t, uint32(2), readSchema(t, dir), "the lock was released and the value kept") | |
| 232 | + | assert.Equal(t, uint32(3), readSchema(t, dir), "the lock was released and the value kept") | |
| 219 | 233 | }) | |
| 220 | 234 | ||
| 221 | 235 | t.Run("garbage and zero are errors", func(t *testing.T) { | |
| @@ -191,12 +191,13 @@ func readSchema(t *testing.T, dir string) uint32 { | |||||
| 191 | func TestSchema(t *testing.T) { | 191 | func TestSchema(t *testing.T) { | ||
| 192 | ctx := context.Background() | 192 | ctx := context.Background() | ||
| 193 | 193 | ||||
| 194 | - | t.Run("missing is written as 1", func(t *testing.T) { | 194 | + | t.Run("missing is written as the current version", func(t *testing.T) { |
| 195 | dir := t.TempDir() | 195 | dir := t.TempDir() | ||
| 196 | s, err := kvstore.Open(ctx, dir) | 196 | s, err := kvstore.Open(ctx, dir) | ||
| 197 | require.NoError(t, err) | 197 | require.NoError(t, err) | ||
| 198 | require.NoError(t, s.Close(ctx)) | 198 | require.NoError(t, s.Close(ctx)) | ||
| 199 | - | assert.Equal(t, uint32(1), readSchema(t, dir)) | 199 | + | assert.Equal(t, uint32(kvstore.SchemaVersion), readSchema(t, dir)) |
| 200 | + | assert.Equal(t, uint32(2), uint32(kvstore.SchemaVersion), "branch prefixes need schema 2") | |||
| 200 | }) | 201 | }) | ||
| 201 | 202 | ||||
| 202 | t.Run("current is accepted and kept", func(t *testing.T) { | 203 | t.Run("current is accepted and kept", func(t *testing.T) { | ||
| @@ -207,15 +208,28 @@ func TestSchema(t *testing.T) { | |||||
| 207 | s, err = kvstore.Open(ctx, dir) | 208 | s, err = kvstore.Open(ctx, dir) | ||
| 208 | require.NoError(t, err) | 209 | require.NoError(t, err) | ||
| 209 | require.NoError(t, s.Close(ctx)) | 210 | require.NoError(t, s.Close(ctx)) | ||
| 210 | - | assert.Equal(t, uint32(1), readSchema(t, dir)) | 211 | + | assert.Equal(t, uint32(kvstore.SchemaVersion), readSchema(t, dir)) |
| 212 | + | }) | |||
| 213 | + | ||||
| 214 | + | t.Run("version 1 is upgraded to 2, its data kept", func(t *testing.T) { | |||
| 215 | + | dir := t.TempDir() | |||
| 216 | + | putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 1})) | |||
| 217 | + | putRaw(t, dir, "repo/old", mustProto(t, &storagepb.Repo{Name: "old", Visibility: storagepb.Visibility_VISIBILITY_PUBLIC})) | |||
| 218 | + | s, err := kvstore.Open(ctx, dir) | |||
| 219 | + | require.NoError(t, err) | |||
| 220 | + | got, err := s.Get(ctx, name(t, "old")) | |||
| 221 | + | require.NoError(t, err) | |||
| 222 | + | assert.Equal(t, "old", got.Name.String()) | |||
| 223 | + | require.NoError(t, s.Close(ctx)) | |||
| 224 | + | assert.Equal(t, uint32(2), readSchema(t, dir)) | |||
| 211 | }) | 225 | }) | ||
| 212 | 226 | ||||
| 213 | t.Run("newer stops startup and releases the lock", func(t *testing.T) { | 227 | t.Run("newer stops startup and releases the lock", func(t *testing.T) { | ||
| 214 | dir := t.TempDir() | 228 | dir := t.TempDir() | ||
| 215 | - | putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 2})) | 229 | + | putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 3})) |
| 216 | _, err := kvstore.Open(ctx, dir) | 230 | _, err := kvstore.Open(ctx, dir) | ||
| 217 | require.ErrorIs(t, err, kvstore.ErrSchemaTooNew) | 231 | require.ErrorIs(t, err, kvstore.ErrSchemaTooNew) | ||
| 218 | - | assert.Equal(t, uint32(2), readSchema(t, dir), "the lock was released and the value kept") | 232 | + | assert.Equal(t, uint32(3), readSchema(t, dir), "the lock was released and the value kept") |
| 219 | }) | 233 | }) | ||
| 220 | 234 | ||||
| 221 | t.Run("garbage and zero are errors", func(t *testing.T) { | 235 | t.Run("garbage and zero are errors", func(t *testing.T) { | ||
M internal/adapters/kvstore/tokens.go +13 -0
| @@ -191,6 +191,11 @@ func (s *Tokens) Touch(ctx context.Context, id string, at time.Time) error { | |||
| 191 | 191 | return fmt.Errorf("touch token %q: %w", id, domain.ErrConflict) | |
| 192 | 192 | } | |
| 193 | 193 | ||
| 194 | + | // encodeToken refuses what the use case already refused (scopes, branch | |
| 195 | + | // prefixes): a malformed prefix must never reach the store, where it would | |
| 196 | + | // limit a token wrongly or not at all. | |
| 197 | + | // | |
| 198 | + | //hexcheck:ignore suspicious-business-logic-in-adapter schema encoding: it re-checks the syntax the use case enforced, it decides no policy | |
| 194 | 199 | func encodeToken(t domain.Token) ([]byte, error) { | |
| 195 | 200 | if !validTokenID(t.ID) { | |
| 196 | 201 | return nil, fmt.Errorf("%w: invalid id %q", domain.ErrInvalidToken, t.ID) | |
| @@ -221,6 +226,12 @@ func encodeToken(t domain.Token) ([]byte, error) { | |||
| 221 | 226 | } | |
| 222 | 227 | pb.Repos = append(pb.Repos, r.String()) | |
| 223 | 228 | } | |
| 229 | + | for _, bp := range t.BranchPrefixes { | |
| 230 | + | if _, err := domain.ParseBranchPrefix(bp); err != nil { | |
| 231 | + | return nil, err | |
| 232 | + | } | |
| 233 | + | pb.BranchPrefixes = append(pb.BranchPrefixes, bp) | |
| 234 | + | } | |
| 224 | 235 | return mustMarshal(pb), nil | |
| 225 | 236 | } | |
| 226 | 237 | ||
| @@ -256,5 +267,7 @@ func decodeToken(val []byte) (domain.Token, error) { | |||
| 256 | 267 | } | |
| 257 | 268 | t.Repos = append(t.Repos, name) | |
| 258 | 269 | } | |
| 270 | + | // Kept as stored: a token with prefixes stays limited, whatever they say. | |
| 271 | + | t.BranchPrefixes = slices.Clone(pb.GetBranchPrefixes()) | |
| 259 | 272 | return t, nil | |
| 260 | 273 | } | |
| @@ -191,6 +191,11 @@ func (s *Tokens) Touch(ctx context.Context, id string, at time.Time) error { | |||||
| 191 | return fmt.Errorf("touch token %q: %w", id, domain.ErrConflict) | 191 | return fmt.Errorf("touch token %q: %w", id, domain.ErrConflict) | ||
| 192 | } | 192 | } | ||
| 193 | 193 | ||||
| 194 | + | // encodeToken refuses what the use case already refused (scopes, branch | |||
| 195 | + | // prefixes): a malformed prefix must never reach the store, where it would | |||
| 196 | + | // limit a token wrongly or not at all. | |||
| 197 | + | // | |||
| 198 | + | //hexcheck:ignore suspicious-business-logic-in-adapter schema encoding: it re-checks the syntax the use case enforced, it decides no policy | |||
| 194 | func encodeToken(t domain.Token) ([]byte, error) { | 199 | func encodeToken(t domain.Token) ([]byte, error) { | ||
| 195 | if !validTokenID(t.ID) { | 200 | if !validTokenID(t.ID) { | ||
| 196 | return nil, fmt.Errorf("%w: invalid id %q", domain.ErrInvalidToken, t.ID) | 201 | return nil, fmt.Errorf("%w: invalid id %q", domain.ErrInvalidToken, t.ID) | ||
| @@ -221,6 +226,12 @@ func encodeToken(t domain.Token) ([]byte, error) { | |||||
| 221 | } | 226 | } | ||
| 222 | pb.Repos = append(pb.Repos, r.String()) | 227 | pb.Repos = append(pb.Repos, r.String()) | ||
| 223 | } | 228 | } | ||
| 229 | + | for _, bp := range t.BranchPrefixes { | |||
| 230 | + | if _, err := domain.ParseBranchPrefix(bp); err != nil { | |||
| 231 | + | return nil, err | |||
| 232 | + | } | |||
| 233 | + | pb.BranchPrefixes = append(pb.BranchPrefixes, bp) | |||
| 234 | + | } | |||
| 224 | return mustMarshal(pb), nil | 235 | return mustMarshal(pb), nil | ||
| 225 | } | 236 | } | ||
| 226 | 237 | ||||
| @@ -256,5 +267,7 @@ func decodeToken(val []byte) (domain.Token, error) { | |||||
| 256 | } | 267 | } | ||
| 257 | t.Repos = append(t.Repos, name) | 268 | t.Repos = append(t.Repos, name) | ||
| 258 | } | 269 | } | ||
| 270 | + | // Kept as stored: a token with prefixes stays limited, whatever they say. | |||
| 271 | + | t.BranchPrefixes = slices.Clone(pb.GetBranchPrefixes()) | |||
| 259 | return t, nil | 272 | return t, nil | ||
| 260 | } | 273 | } | ||
M internal/adapters/kvstore/tokens_test.go +53 -0
| @@ -12,6 +12,7 @@ import ( | |||
| 12 | 12 | "github.com/stretchr/testify/assert" | |
| 13 | 13 | "github.com/stretchr/testify/require" | |
| 14 | 14 | ||
| 15 | + | storagepb "bnema.dev/ermite/gen/ermite/storage" | |
| 15 | 16 | "bnema.dev/ermite/internal/domain" | |
| 16 | 17 | ) | |
| 17 | 18 | ||
| @@ -192,3 +193,55 @@ func TestTokensConcurrentTouchAndRevoke(t *testing.T) { | |||
| 192 | 193 | assert.Len(t, list, 1) | |
| 193 | 194 | } | |
| 194 | 195 | } | |
| 196 | + | ||
| 197 | + | func TestTokensBranchPrefixes(t *testing.T) { | |
| 198 | + | ctx := context.Background() | |
| 199 | + | ||
| 200 | + | t.Run("round trip, order kept", func(t *testing.T) { | |
| 201 | + | s := openStore(t, t.TempDir()) | |
| 202 | + | tok := newToken(t, "t1", "s1") | |
| 203 | + | tok.BranchPrefixes = []string{"b/", "agent/claude/"} | |
| 204 | + | require.NoError(t, s.Tokens().Create(ctx, tok)) | |
| 205 | + | got, err := s.Tokens().Get(ctx, "t1") | |
| 206 | + | require.NoError(t, err) | |
| 207 | + | assert.Equal(t, tok, got) | |
| 208 | + | byHash, err := s.Tokens().FindByHash(ctx, tok.Hash) | |
| 209 | + | require.NoError(t, err) | |
| 210 | + | assert.Equal(t, []string{"b/", "agent/claude/"}, byHash.BranchPrefixes) | |
| 211 | + | }) | |
| 212 | + | ||
| 213 | + | t.Run("a token stored before the field existed decodes with no prefixes", func(t *testing.T) { | |
| 214 | + | dir := t.TempDir() | |
| 215 | + | old := newToken(t, "old", "s-old") | |
| 216 | + | putRaw(t, dir, "token/old", mustProto(t, &storagepb.Token{ | |
| 217 | + | Id: "old", Label: "old", Hash: old.Hash[:], Scopes: []string{"repo:write"}, | |
| 218 | + | })) | |
| 219 | + | s := openStore(t, dir) | |
| 220 | + | got, err := s.Tokens().Get(ctx, "old") | |
| 221 | + | require.NoError(t, err) | |
| 222 | + | assert.Empty(t, got.BranchPrefixes) | |
| 223 | + | assert.Equal(t, []domain.Scope{domain.ScopeRepoWrite}, got.Scopes) | |
| 224 | + | }) | |
| 225 | + | ||
| 226 | + | t.Run("an invalid prefix is not stored", func(t *testing.T) { | |
| 227 | + | s := openStore(t, t.TempDir()) | |
| 228 | + | for _, bad := range []string{"", "agent", "/", "a b/"} { | |
| 229 | + | tok := newToken(t, "t1", "s1") | |
| 230 | + | tok.BranchPrefixes = []string{bad} | |
| 231 | + | assert.ErrorIs(t, s.Tokens().Create(ctx, tok), domain.ErrInvalidToken, "%q", bad) | |
| 232 | + | } | |
| 233 | + | _, err := s.Tokens().Get(ctx, "t1") | |
| 234 | + | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 235 | + | }) | |
| 236 | + | ||
| 237 | + | t.Run("a stored prefix survives touch and stays limiting", func(t *testing.T) { | |
| 238 | + | s := openStore(t, t.TempDir()) | |
| 239 | + | tok := newToken(t, "t1", "s1") | |
| 240 | + | tok.BranchPrefixes = []string{"agent/"} | |
| 241 | + | require.NoError(t, s.Tokens().Create(ctx, tok)) | |
| 242 | + | require.NoError(t, s.Tokens().Touch(ctx, "t1", created.Add(time.Hour))) | |
| 243 | + | got, err := s.Tokens().Get(ctx, "t1") | |
| 244 | + | require.NoError(t, err) | |
| 245 | + | assert.Equal(t, []string{"agent/"}, got.BranchPrefixes) | |
| 246 | + | }) | |
| 247 | + | } | |
| @@ -12,6 +12,7 @@ import ( | |||||
| 12 | "github.com/stretchr/testify/assert" | 12 | "github.com/stretchr/testify/assert" | ||
| 13 | "github.com/stretchr/testify/require" | 13 | "github.com/stretchr/testify/require" | ||
| 14 | 14 | ||||
| 15 | + | storagepb "bnema.dev/ermite/gen/ermite/storage" | |||
| 15 | "bnema.dev/ermite/internal/domain" | 16 | "bnema.dev/ermite/internal/domain" | ||
| 16 | ) | 17 | ) | ||
| 17 | 18 | ||||
| @@ -192,3 +193,55 @@ func TestTokensConcurrentTouchAndRevoke(t *testing.T) { | |||||
| 192 | assert.Len(t, list, 1) | 193 | assert.Len(t, list, 1) | ||
| 193 | } | 194 | } | ||
| 194 | } | 195 | } | ||
| 196 | + | ||||
| 197 | + | func TestTokensBranchPrefixes(t *testing.T) { | |||
| 198 | + | ctx := context.Background() | |||
| 199 | + | ||||
| 200 | + | t.Run("round trip, order kept", func(t *testing.T) { | |||
| 201 | + | s := openStore(t, t.TempDir()) | |||
| 202 | + | tok := newToken(t, "t1", "s1") | |||
| 203 | + | tok.BranchPrefixes = []string{"b/", "agent/claude/"} | |||
| 204 | + | require.NoError(t, s.Tokens().Create(ctx, tok)) | |||
| 205 | + | got, err := s.Tokens().Get(ctx, "t1") | |||
| 206 | + | require.NoError(t, err) | |||
| 207 | + | assert.Equal(t, tok, got) | |||
| 208 | + | byHash, err := s.Tokens().FindByHash(ctx, tok.Hash) | |||
| 209 | + | require.NoError(t, err) | |||
| 210 | + | assert.Equal(t, []string{"b/", "agent/claude/"}, byHash.BranchPrefixes) | |||
| 211 | + | }) | |||
| 212 | + | ||||
| 213 | + | t.Run("a token stored before the field existed decodes with no prefixes", func(t *testing.T) { | |||
| 214 | + | dir := t.TempDir() | |||
| 215 | + | old := newToken(t, "old", "s-old") | |||
| 216 | + | putRaw(t, dir, "token/old", mustProto(t, &storagepb.Token{ | |||
| 217 | + | Id: "old", Label: "old", Hash: old.Hash[:], Scopes: []string{"repo:write"}, | |||
| 218 | + | })) | |||
| 219 | + | s := openStore(t, dir) | |||
| 220 | + | got, err := s.Tokens().Get(ctx, "old") | |||
| 221 | + | require.NoError(t, err) | |||
| 222 | + | assert.Empty(t, got.BranchPrefixes) | |||
| 223 | + | assert.Equal(t, []domain.Scope{domain.ScopeRepoWrite}, got.Scopes) | |||
| 224 | + | }) | |||
| 225 | + | ||||
| 226 | + | t.Run("an invalid prefix is not stored", func(t *testing.T) { | |||
| 227 | + | s := openStore(t, t.TempDir()) | |||
| 228 | + | for _, bad := range []string{"", "agent", "/", "a b/"} { | |||
| 229 | + | tok := newToken(t, "t1", "s1") | |||
| 230 | + | tok.BranchPrefixes = []string{bad} | |||
| 231 | + | assert.ErrorIs(t, s.Tokens().Create(ctx, tok), domain.ErrInvalidToken, "%q", bad) | |||
| 232 | + | } | |||
| 233 | + | _, err := s.Tokens().Get(ctx, "t1") | |||
| 234 | + | assert.ErrorIs(t, err, domain.ErrNotFound) | |||
| 235 | + | }) | |||
| 236 | + | ||||
| 237 | + | t.Run("a stored prefix survives touch and stays limiting", func(t *testing.T) { | |||
| 238 | + | s := openStore(t, t.TempDir()) | |||
| 239 | + | tok := newToken(t, "t1", "s1") | |||
| 240 | + | tok.BranchPrefixes = []string{"agent/"} | |||
| 241 | + | require.NoError(t, s.Tokens().Create(ctx, tok)) | |||
| 242 | + | require.NoError(t, s.Tokens().Touch(ctx, "t1", created.Add(time.Hour))) | |||
| 243 | + | got, err := s.Tokens().Get(ctx, "t1") | |||
| 244 | + | require.NoError(t, err) | |||
| 245 | + | assert.Equal(t, []string{"agent/"}, got.BranchPrefixes) | |||
| 246 | + | }) | |||
| 247 | + | } | |||
M internal/domain/principal.go +3 -1
| @@ -129,7 +129,9 @@ func (p Principal) AllowsRef(ref string) bool { | |||
| 129 | 129 | return false | |
| 130 | 130 | } | |
| 131 | 131 | for _, prefix := range p.branchPrefixes { | |
| 132 | - | if strings.HasPrefix(name, prefix) { | |
| 132 | + | // An empty prefix, which ParseBranchPrefix never lets in, matches | |
| 133 | + | // nothing instead of everything: a limited token fails closed. | |
| 134 | + | if prefix != "" && strings.HasPrefix(name, prefix) { | |
| 133 | 135 | return true | |
| 134 | 136 | } | |
| 135 | 137 | } | |
| @@ -129,7 +129,9 @@ func (p Principal) AllowsRef(ref string) bool { | |||||
| 129 | return false | 129 | return false | ||
| 130 | } | 130 | } | ||
| 131 | for _, prefix := range p.branchPrefixes { | 131 | for _, prefix := range p.branchPrefixes { | ||
| 132 | - | if strings.HasPrefix(name, prefix) { | 132 | + | // An empty prefix, which ParseBranchPrefix never lets in, matches |
| 133 | + | // nothing instead of everything: a limited token fails closed. | |||
| 134 | + | if prefix != "" && strings.HasPrefix(name, prefix) { | |||
| 133 | return true | 135 | return true | ||
| 134 | } | 136 | } | ||
| 135 | } | 137 | } | ||
M internal/domain/token_test.go +4 -0
| @@ -186,6 +186,10 @@ func TestAllowsRef(t *testing.T) { | |||
| 186 | 186 | assert.Equal(t, want, limited.AllowsRef(ref), "%q", ref) | |
| 187 | 187 | } | |
| 188 | 188 | ||
| 189 | + | // A corrupt empty prefix still limits the token and allows nothing. | |
| 190 | + | corrupt := domain.TokenPrincipal(domain.Token{Scopes: []domain.Scope{domain.ScopeRepoWrite}, BranchPrefixes: []string{""}}) | |
| 191 | + | assert.False(t, corrupt.AllowsRef("refs/heads/main")) | |
| 192 | + | ||
| 189 | 193 | // Without prefixes every ref is allowed. | |
| 190 | 194 | unlimited := domain.TokenPrincipal(domain.Token{Scopes: []domain.Scope{domain.ScopeRepoWrite}}) | |
| 191 | 195 | assert.True(t, unlimited.AllowsRef("refs/tags/v1")) | |
| @@ -186,6 +186,10 @@ func TestAllowsRef(t *testing.T) { | |||||
| 186 | assert.Equal(t, want, limited.AllowsRef(ref), "%q", ref) | 186 | assert.Equal(t, want, limited.AllowsRef(ref), "%q", ref) | ||
| 187 | } | 187 | } | ||
| 188 | 188 | ||||
| 189 | + | // A corrupt empty prefix still limits the token and allows nothing. | |||
| 190 | + | corrupt := domain.TokenPrincipal(domain.Token{Scopes: []domain.Scope{domain.ScopeRepoWrite}, BranchPrefixes: []string{""}}) | |||
| 191 | + | assert.False(t, corrupt.AllowsRef("refs/heads/main")) | |||
| 192 | + | ||||
| 189 | // Without prefixes every ref is allowed. | 193 | // Without prefixes every ref is allowed. | ||
| 190 | unlimited := domain.TokenPrincipal(domain.Token{Scopes: []domain.Scope{domain.ScopeRepoWrite}}) | 194 | unlimited := domain.TokenPrincipal(domain.Token{Scopes: []domain.Scope{domain.ScopeRepoWrite}}) | ||
| 191 | assert.True(t, unlimited.AllowsRef("refs/tags/v1")) | 195 | assert.True(t, unlimited.AllowsRef("refs/tags/v1")) | ||
M proto/ermite/storage/repo.proto +3 -0
| @@ -53,4 +53,7 @@ message Token { | |||
| 53 | 53 | google.protobuf.Timestamp created_at = 7; | |
| 54 | 54 | // Unset until the token is first used. | |
| 55 | 55 | google.protobuf.Timestamp last_used_at = 8; | |
| 56 | + | // Branch prefixes, each ending with "/", that the token may write; empty | |
| 57 | + | // means every branch. Added in storage schema 2. | |
| 58 | + | repeated string branch_prefixes = 9; | |
| 56 | 59 | } | |
| @@ -53,4 +53,7 @@ message Token { | |||||
| 53 | google.protobuf.Timestamp created_at = 7; | 53 | google.protobuf.Timestamp created_at = 7; | ||
| 54 | // Unset until the token is first used. | 54 | // Unset until the token is first used. | ||
| 55 | google.protobuf.Timestamp last_used_at = 8; | 55 | google.protobuf.Timestamp last_used_at = 8; | ||
| 56 | + | // Branch prefixes, each ending with "/", that the token may write; empty | |||
| 57 | + | // means every branch. Added in storage schema 2. | |||
| 58 | + | repeated string branch_prefixes = 9; | |||
| 56 | } | 59 | } | ||