refactor(app): drop dead crypto/rand error paths and unused helpers
crypto/rand.Read never fails, so newSecret and newTokenID return strings. Replace compareNames with strings.Compare, remove Principal.IsToken, the duplicate TokenStore assertion in the e2e tests and a no-op assertion.
- commit
- a771dd253a2c37da15c80068a3493c2544b8a5c6
- author
- brice · · 2026-10-10 08:00 UTC
- parent
- 89fd514
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +13 -42
M cmd/ermite/e2e_test.go +0 -2
| @@ -22,7 +22,6 @@ import ( | |||
| 22 | 22 | ||
| 23 | 23 | "bnema.dev/ermite/internal/app" | |
| 24 | 24 | "bnema.dev/ermite/internal/domain" | |
| 25 | - | "bnema.dev/ermite/internal/ports" | |
| 26 | 25 | ) | |
| 27 | 26 | ||
| 28 | 27 | func scopes(s ...domain.Scope) []domain.Scope { return s } | |
| @@ -598,7 +597,6 @@ func TestAC8TokenCreationThroughTheForge(t *testing.T) { | |||
| 598 | 597 | assert.Equal(t, admin.Token.ID, list[0].ID) | |
| 599 | 598 | _, err = h.forge.store.Tokens().FindByHash(ctx, child.Token.Hash) | |
| 600 | 599 | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 601 | - | var _ ports.TokenStore = h.forge.store.Tokens() | |
| 602 | 600 | } | |
| 603 | 601 | ||
| 604 | 602 | // Large pushes arrive chunked (git's http.postBuffer is 1 MiB): the handler | |
| @@ -22,7 +22,6 @@ import ( | |||||
| 22 | 22 | ||||
| 23 | "bnema.dev/ermite/internal/app" | 23 | "bnema.dev/ermite/internal/app" | ||
| 24 | "bnema.dev/ermite/internal/domain" | 24 | "bnema.dev/ermite/internal/domain" | ||
| 25 | - | "bnema.dev/ermite/internal/ports" | |||
| 26 | ) | 25 | ) | ||
| 27 | 26 | ||||
| 28 | func scopes(s ...domain.Scope) []domain.Scope { return s } | 27 | func scopes(s ...domain.Scope) []domain.Scope { return s } | ||
| @@ -598,7 +597,6 @@ func TestAC8TokenCreationThroughTheForge(t *testing.T) { | |||||
| 598 | assert.Equal(t, admin.Token.ID, list[0].ID) | 597 | assert.Equal(t, admin.Token.ID, list[0].ID) | ||
| 599 | _, err = h.forge.store.Tokens().FindByHash(ctx, child.Token.Hash) | 598 | _, err = h.forge.store.Tokens().FindByHash(ctx, child.Token.Hash) | ||
| 600 | assert.ErrorIs(t, err, domain.ErrNotFound) | 599 | assert.ErrorIs(t, err, domain.ErrNotFound) | ||
| 601 | - | var _ ports.TokenStore = h.forge.store.Tokens() | |||
| 602 | } | 600 | } | ||
| 603 | 601 | ||||
| 604 | // Large pushes arrive chunked (git's http.postBuffer is 1 MiB): the handler | 602 | // Large pushes arrive chunked (git's http.postBuffer is 1 MiB): the handler | ||
M internal/app/maintenance_test.go +0 -2
| @@ -11,7 +11,6 @@ import ( | |||
| 11 | 11 | "time" | |
| 12 | 12 | ||
| 13 | 13 | "github.com/stretchr/testify/assert" | |
| 14 | - | "github.com/stretchr/testify/require" | |
| 15 | 14 | ||
| 16 | 15 | "bnema.dev/ermite/internal/app" | |
| 17 | 16 | "bnema.dev/ermite/internal/domain" | |
| @@ -170,5 +169,4 @@ func TestMaintainerConcurrentSchedule(t *testing.T) { | |||
| 170 | 169 | } | |
| 171 | 170 | wg.Wait() | |
| 172 | 171 | m.Close(ctx) | |
| 173 | - | require.True(t, true) | |
| 174 | 172 | } | |
| @@ -11,7 +11,6 @@ import ( | |||||
| 11 | "time" | 11 | "time" | ||
| 12 | 12 | ||||
| 13 | "github.com/stretchr/testify/assert" | 13 | "github.com/stretchr/testify/assert" | ||
| 14 | - | "github.com/stretchr/testify/require" | |||
| 15 | 14 | ||||
| 16 | "bnema.dev/ermite/internal/app" | 15 | "bnema.dev/ermite/internal/app" | ||
| 17 | "bnema.dev/ermite/internal/domain" | 16 | "bnema.dev/ermite/internal/domain" | ||
| @@ -170,5 +169,4 @@ func TestMaintainerConcurrentSchedule(t *testing.T) { | |||||
| 170 | } | 169 | } | ||
| 171 | wg.Wait() | 170 | wg.Wait() | ||
| 172 | m.Close(ctx) | 171 | m.Close(ctx) | ||
| 173 | - | require.True(t, true) | |||
| 174 | } | 172 | } | ||
M internal/app/tokens.go +13 -32
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "errors" | |
| 12 | 12 | "fmt" | |
| 13 | 13 | "slices" | |
| 14 | + | "strings" | |
| 14 | 15 | "time" | |
| 15 | 16 | "unicode/utf8" | |
| 16 | 17 | ||
| @@ -81,16 +82,9 @@ func (t *Tokens) Create(ctx context.Context, p domain.Principal, params CreateTo | |||
| 81 | 82 | } | |
| 82 | 83 | ||
| 83 | 84 | for range 3 { | |
| 84 | - | secret, err := newSecret() | |
| 85 | - | if err != nil { | |
| 86 | - | return CreatedToken{}, err | |
| 87 | - | } | |
| 88 | - | id, err := newTokenID() | |
| 89 | - | if err != nil { | |
| 90 | - | return CreatedToken{}, err | |
| 91 | - | } | |
| 92 | - | tok.ID, tok.Hash = id, domain.HashTokenSecret(secret) | |
| 93 | - | err = t.store.Create(ctx, tok) | |
| 85 | + | secret := newSecret() | |
| 86 | + | tok.ID, tok.Hash = newTokenID(), domain.HashTokenSecret(secret) | |
| 87 | + | err := t.store.Create(ctx, tok) | |
| 94 | 88 | if errors.Is(err, domain.ErrAlreadyExists) { | |
| 95 | 89 | continue // a random id or secret collided: practically never | |
| 96 | 90 | } | |
| @@ -145,20 +139,10 @@ func buildToken(params CreateTokenParams, now time.Time) (domain.Token, error) { | |||
| 145 | 139 | tok.Repos = append(tok.Repos, r) | |
| 146 | 140 | } | |
| 147 | 141 | } | |
| 148 | - | slices.SortFunc(tok.Repos, func(a, b domain.RepoName) int { return compareNames(a, b) }) | |
| 142 | + | slices.SortFunc(tok.Repos, func(a, b domain.RepoName) int { return strings.Compare(a.String(), b.String()) }) | |
| 149 | 143 | return tok, nil | |
| 150 | 144 | } | |
| 151 | 145 | ||
| 152 | - | func compareNames(a, b domain.RepoName) int { | |
| 153 | - | switch { | |
| 154 | - | case a.String() < b.String(): | |
| 155 | - | return -1 | |
| 156 | - | case a.String() > b.String(): | |
| 157 | - | return 1 | |
| 158 | - | } | |
| 159 | - | return 0 | |
| 160 | - | } | |
| 161 | - | ||
| 162 | 146 | // checkNoEscalation enforces that tok is within the rights of its creator. | |
| 163 | 147 | func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |
| 164 | 148 | if p.IsRoot() { | |
| @@ -219,22 +203,19 @@ func requireTokenAdmin(p domain.Principal) error { | |||
| 219 | 203 | return nil | |
| 220 | 204 | } | |
| 221 | 205 | ||
| 222 | - | // newSecret returns "ermite_" and 32 random bytes in base64url. | |
| 223 | - | func newSecret() (string, error) { | |
| 206 | + | // newSecret returns "ermite_" and 32 random bytes in base64url. crypto/rand | |
| 207 | + | // does not fail: it aborts the program if the system has no entropy. | |
| 208 | + | func newSecret() string { | |
| 224 | 209 | var raw [32]byte | |
| 225 | - | if _, err := rand.Read(raw[:]); err != nil { | |
| 226 | - | return "", fmt.Errorf("random token secret: %w", err) | |
| 227 | - | } | |
| 228 | - | return domain.TokenSecretPrefix + base64.RawURLEncoding.EncodeToString(raw[:]), nil | |
| 210 | + | _, _ = rand.Read(raw[:]) | |
| 211 | + | return domain.TokenSecretPrefix + base64.RawURLEncoding.EncodeToString(raw[:]) | |
| 229 | 212 | } | |
| 230 | 213 | ||
| 231 | 214 | // newTokenID returns 16 hex characters. | |
| 232 | - | func newTokenID() (string, error) { | |
| 215 | + | func newTokenID() string { | |
| 233 | 216 | var raw [8]byte | |
| 234 | - | if _, err := rand.Read(raw[:]); err != nil { | |
| 235 | - | return "", fmt.Errorf("random token id: %w", err) | |
| 236 | - | } | |
| 237 | - | return hex.EncodeToString(raw[:]), nil | |
| 217 | + | _, _ = rand.Read(raw[:]) | |
| 218 | + | return hex.EncodeToString(raw[:]) | |
| 238 | 219 | } | |
| 239 | 220 | ||
| 240 | 221 | // Auth authenticates secrets presented by callers. | |
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | "errors" | 11 | "errors" | ||
| 12 | "fmt" | 12 | "fmt" | ||
| 13 | "slices" | 13 | "slices" | ||
| 14 | + | "strings" | |||
| 14 | "time" | 15 | "time" | ||
| 15 | "unicode/utf8" | 16 | "unicode/utf8" | ||
| 16 | 17 | ||||
| @@ -81,16 +82,9 @@ func (t *Tokens) Create(ctx context.Context, p domain.Principal, params CreateTo | |||||
| 81 | } | 82 | } | ||
| 82 | 83 | ||||
| 83 | for range 3 { | 84 | for range 3 { | ||
| 84 | - | secret, err := newSecret() | 85 | + | secret := newSecret() |
| 85 | - | if err != nil { | 86 | + | tok.ID, tok.Hash = newTokenID(), domain.HashTokenSecret(secret) |
| 86 | - | return CreatedToken{}, err | 87 | + | err := t.store.Create(ctx, tok) |
| 87 | - | } | |||
| 88 | - | id, err := newTokenID() | |||
| 89 | - | if err != nil { | |||
| 90 | - | return CreatedToken{}, err | |||
| 91 | - | } | |||
| 92 | - | tok.ID, tok.Hash = id, domain.HashTokenSecret(secret) | |||
| 93 | - | err = t.store.Create(ctx, tok) | |||
| 94 | if errors.Is(err, domain.ErrAlreadyExists) { | 88 | if errors.Is(err, domain.ErrAlreadyExists) { | ||
| 95 | continue // a random id or secret collided: practically never | 89 | continue // a random id or secret collided: practically never | ||
| 96 | } | 90 | } | ||
| @@ -145,20 +139,10 @@ func buildToken(params CreateTokenParams, now time.Time) (domain.Token, error) { | |||||
| 145 | tok.Repos = append(tok.Repos, r) | 139 | tok.Repos = append(tok.Repos, r) | ||
| 146 | } | 140 | } | ||
| 147 | } | 141 | } | ||
| 148 | - | slices.SortFunc(tok.Repos, func(a, b domain.RepoName) int { return compareNames(a, b) }) | 142 | + | slices.SortFunc(tok.Repos, func(a, b domain.RepoName) int { return strings.Compare(a.String(), b.String()) }) |
| 149 | return tok, nil | 143 | return tok, nil | ||
| 150 | } | 144 | } | ||
| 151 | 145 | ||||
| 152 | - | func compareNames(a, b domain.RepoName) int { | |||
| 153 | - | switch { | |||
| 154 | - | case a.String() < b.String(): | |||
| 155 | - | return -1 | |||
| 156 | - | case a.String() > b.String(): | |||
| 157 | - | return 1 | |||
| 158 | - | } | |||
| 159 | - | return 0 | |||
| 160 | - | } | |||
| 161 | - | ||||
| 162 | // checkNoEscalation enforces that tok is within the rights of its creator. | 146 | // checkNoEscalation enforces that tok is within the rights of its creator. | ||
| 163 | func checkNoEscalation(p domain.Principal, tok domain.Token) error { | 147 | func checkNoEscalation(p domain.Principal, tok domain.Token) error { | ||
| 164 | if p.IsRoot() { | 148 | if p.IsRoot() { | ||
| @@ -219,22 +203,19 @@ func requireTokenAdmin(p domain.Principal) error { | |||||
| 219 | return nil | 203 | return nil | ||
| 220 | } | 204 | } | ||
| 221 | 205 | ||||
| 222 | - | // newSecret returns "ermite_" and 32 random bytes in base64url. | 206 | + | // newSecret returns "ermite_" and 32 random bytes in base64url. crypto/rand |
| 223 | - | func newSecret() (string, error) { | 207 | + | // does not fail: it aborts the program if the system has no entropy. |
| 208 | + | func newSecret() string { | |||
| 224 | var raw [32]byte | 209 | var raw [32]byte | ||
| 225 | - | if _, err := rand.Read(raw[:]); err != nil { | 210 | + | _, _ = rand.Read(raw[:]) |
| 226 | - | return "", fmt.Errorf("random token secret: %w", err) | 211 | + | return domain.TokenSecretPrefix + base64.RawURLEncoding.EncodeToString(raw[:]) |
| 227 | - | } | |||
| 228 | - | return domain.TokenSecretPrefix + base64.RawURLEncoding.EncodeToString(raw[:]), nil | |||
| 229 | } | 212 | } | ||
| 230 | 213 | ||||
| 231 | // newTokenID returns 16 hex characters. | 214 | // newTokenID returns 16 hex characters. | ||
| 232 | - | func newTokenID() (string, error) { | 215 | + | func newTokenID() string { |
| 233 | var raw [8]byte | 216 | var raw [8]byte | ||
| 234 | - | if _, err := rand.Read(raw[:]); err != nil { | 217 | + | _, _ = rand.Read(raw[:]) |
| 235 | - | return "", fmt.Errorf("random token id: %w", err) | 218 | + | return hex.EncodeToString(raw[:]) |
| 236 | - | } | |||
| 237 | - | return hex.EncodeToString(raw[:]), nil | |||
| 238 | } | 219 | } | ||
| 239 | 220 | ||||
| 240 | // Auth authenticates secrets presented by callers. | 221 | // Auth authenticates secrets presented by callers. | ||
M internal/app/tokens_test.go +0 -1
| @@ -256,7 +256,6 @@ func TestAuthenticate(t *testing.T) { | |||
| 256 | 256 | f.store.EXPECT().Touch(ctx, "t1", now).Return(nil) | |
| 257 | 257 | p, err := f.auth.Authenticate(ctx, secret) | |
| 258 | 258 | require.NoError(t, err) | |
| 259 | - | assert.True(t, p.IsToken()) | |
| 260 | 259 | assert.Equal(t, "t1", p.TokenID()) | |
| 261 | 260 | assert.True(t, p.Allows(domain.ScopeRepoWrite, name(t, "a"))) | |
| 262 | 261 | assert.False(t, p.Allows(domain.ScopeRepoWrite, name(t, "b"))) | |
| @@ -256,7 +256,6 @@ func TestAuthenticate(t *testing.T) { | |||||
| 256 | f.store.EXPECT().Touch(ctx, "t1", now).Return(nil) | 256 | f.store.EXPECT().Touch(ctx, "t1", now).Return(nil) | ||
| 257 | p, err := f.auth.Authenticate(ctx, secret) | 257 | p, err := f.auth.Authenticate(ctx, secret) | ||
| 258 | require.NoError(t, err) | 258 | require.NoError(t, err) | ||
| 259 | - | assert.True(t, p.IsToken()) | |||
| 260 | assert.Equal(t, "t1", p.TokenID()) | 259 | assert.Equal(t, "t1", p.TokenID()) | ||
| 261 | assert.True(t, p.Allows(domain.ScopeRepoWrite, name(t, "a"))) | 260 | assert.True(t, p.Allows(domain.ScopeRepoWrite, name(t, "a"))) | ||
| 262 | assert.False(t, p.Allows(domain.ScopeRepoWrite, name(t, "b"))) | 261 | assert.False(t, p.Allows(domain.ScopeRepoWrite, name(t, "b"))) | ||
M internal/domain/name_test.go +0 -1
| @@ -109,5 +109,4 @@ func TestPrincipal(t *testing.T) { | |||
| 109 | 109 | assert.False(t, zero.IsRoot()) | |
| 110 | 110 | assert.True(t, domain.Root().IsRoot()) | |
| 111 | 111 | assert.Equal(t, "anonymous", domain.Anonymous().Label()) | |
| 112 | - | assert.False(t, domain.Root().IsToken()) | |
| 113 | 112 | } | |
| @@ -109,5 +109,4 @@ func TestPrincipal(t *testing.T) { | |||||
| 109 | assert.False(t, zero.IsRoot()) | 109 | assert.False(t, zero.IsRoot()) | ||
| 110 | assert.True(t, domain.Root().IsRoot()) | 110 | assert.True(t, domain.Root().IsRoot()) | ||
| 111 | assert.Equal(t, "anonymous", domain.Anonymous().Label()) | 111 | assert.Equal(t, "anonymous", domain.Anonymous().Label()) | ||
| 112 | - | assert.False(t, domain.Root().IsToken()) | |||
| 113 | } | 112 | } | ||
M internal/domain/principal.go +0 -3
| @@ -56,9 +56,6 @@ func (p Principal) Label() string { | |||
| 56 | 56 | return "anonymous" | |
| 57 | 57 | } | |
| 58 | 58 | ||
| 59 | - | // IsToken reports whether p holds a stored token. | |
| 60 | - | func (p Principal) IsToken() bool { return p.kind == kindToken } | |
| 61 | - | ||
| 62 | 59 | // IsRoot reports whether p may do everything. | |
| 63 | 60 | func (p Principal) IsRoot() bool { return p.kind == kindRoot } | |
| 64 | 61 | ||
| @@ -56,9 +56,6 @@ func (p Principal) Label() string { | |||||
| 56 | return "anonymous" | 56 | return "anonymous" | ||
| 57 | } | 57 | } | ||
| 58 | 58 | ||||
| 59 | - | // IsToken reports whether p holds a stored token. | |||
| 60 | - | func (p Principal) IsToken() bool { return p.kind == kindToken } | |||
| 61 | - | ||||
| 62 | // IsRoot reports whether p may do everything. | 59 | // IsRoot reports whether p may do everything. | ||
| 63 | func (p Principal) IsRoot() bool { return p.kind == kindRoot } | 60 | func (p Principal) IsRoot() bool { return p.kind == kindRoot } | ||
| 64 | 61 | ||||
M internal/domain/token_test.go +0 -1
| @@ -48,7 +48,6 @@ func TestPrincipalScopes(t *testing.T) { | |||
| 48 | 48 | tok := domain.Token{ID: "t1", Scopes: []domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite}, Repos: []domain.RepoName{a}} | |
| 49 | 49 | p := domain.TokenPrincipal(tok) | |
| 50 | 50 | ||
| 51 | - | assert.True(t, p.IsToken()) | |
| 52 | 51 | assert.Equal(t, "token", p.Label()) | |
| 53 | 52 | assert.Equal(t, "t1", p.TokenID()) | |
| 54 | 53 | assert.False(t, p.IsRoot()) | |
| @@ -48,7 +48,6 @@ func TestPrincipalScopes(t *testing.T) { | |||||
| 48 | tok := domain.Token{ID: "t1", Scopes: []domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite}, Repos: []domain.RepoName{a}} | 48 | tok := domain.Token{ID: "t1", Scopes: []domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite}, Repos: []domain.RepoName{a}} | ||
| 49 | p := domain.TokenPrincipal(tok) | 49 | p := domain.TokenPrincipal(tok) | ||
| 50 | 50 | ||||
| 51 | - | assert.True(t, p.IsToken()) | |||
| 52 | assert.Equal(t, "token", p.Label()) | 51 | assert.Equal(t, "token", p.Label()) | ||
| 53 | assert.Equal(t, "t1", p.TokenID()) | 52 | assert.Equal(t, "t1", p.TokenID()) | ||
| 54 | assert.False(t, p.IsRoot()) | 53 | assert.False(t, p.IsRoot()) | ||