fix(usecase): ignore empty creator prefixes in the no-escalation check
An empty prefix cannot be stored through the use case, but a corrupt record must not let a token create children with any prefix. Also tests that revoking by a branch-limited token is bounded by its prefixes.
- commit
- 5efe4139de3e230fa1e46574385596516dd8689b
- author
- brice · · 2026-10-12 15:16 UTC
- parent
- 12d3d1e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +38 -1
M internal/usecase/tokens.go +1 -1
| @@ -175,7 +175,7 @@ func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |||
| 175 | 175 | return fmt.Errorf("%w: the creating token is limited to branch prefixes, so the new token must be too", domain.ErrForbidden) | |
| 176 | 176 | } | |
| 177 | 177 | for _, bp := range tok.BranchPrefixes { | |
| 178 | - | if !slices.ContainsFunc(own, func(o string) bool { return strings.HasPrefix(bp, o) }) { | |
| 178 | + | if !slices.ContainsFunc(own, func(o string) bool { return o != "" && strings.HasPrefix(bp, o) }) { | |
| 179 | 179 | return fmt.Errorf("%w: branch prefix %s is outside the creating token", domain.ErrForbidden, bp) | |
| 180 | 180 | } | |
| 181 | 181 | } | |
| @@ -175,7 +175,7 @@ func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |||||
| 175 | return fmt.Errorf("%w: the creating token is limited to branch prefixes, so the new token must be too", domain.ErrForbidden) | 175 | return fmt.Errorf("%w: the creating token is limited to branch prefixes, so the new token must be too", domain.ErrForbidden) | ||
| 176 | } | 176 | } | ||
| 177 | for _, bp := range tok.BranchPrefixes { | 177 | for _, bp := range tok.BranchPrefixes { | ||
| 178 | - | if !slices.ContainsFunc(own, func(o string) bool { return strings.HasPrefix(bp, o) }) { | 178 | + | if !slices.ContainsFunc(own, func(o string) bool { return o != "" && strings.HasPrefix(bp, o) }) { |
| 179 | return fmt.Errorf("%w: branch prefix %s is outside the creating token", domain.ErrForbidden, bp) | 179 | return fmt.Errorf("%w: branch prefix %s is outside the creating token", domain.ErrForbidden, bp) | ||
| 180 | } | 180 | } | ||
| 181 | } | 181 | } | ||
M internal/usecase/tokens_test.go +37 -0
| @@ -620,6 +620,43 @@ func TestTokensCreateBranchPrefixes(t *testing.T) { | |||
| 620 | 620 | assert.False(t, p.HasOwnerRights()) | |
| 621 | 621 | }) | |
| 622 | 622 | ||
| 623 | + | t.Run("a corrupt empty creator prefix does not cover every child prefix", func(t *testing.T) { | |
| 624 | + | t.Parallel() | |
| 625 | + | f := newTokenFixture(t) // the store expects no Create | |
| 626 | + | corrupt := domain.TokenPrincipal(domain.Token{ | |
| 627 | + | ID: "c", Scopes: scopes(domain.ScopeTokenAdmin, domain.ScopeRepoWrite), BranchPrefixes: []string{""}, | |
| 628 | + | }) | |
| 629 | + | _, err := f.tokens.Create(ctx, corrupt, ports.CreateTokenParams{ | |
| 630 | + | Label: "x", Scopes: scopes(domain.ScopeRepoWrite), BranchPrefixes: []string{"main/"}, | |
| 631 | + | }) | |
| 632 | + | assert.ErrorIs(t, err, domain.ErrForbidden) | |
| 633 | + | }) | |
| 634 | + | ||
| 635 | + | t.Run("revoking is bounded by the prefixes of a limited token", func(t *testing.T) { | |
| 636 | + | t.Parallel() | |
| 637 | + | ctx := context.Background() | |
| 638 | + | creator := domain.TokenPrincipal(domain.Token{ | |
| 639 | + | ID: "c", Scopes: scopes(domain.ScopeTokenAdmin, domain.ScopeRepoRead), BranchPrefixes: []string{"agent/claude/"}, | |
| 640 | + | }) | |
| 641 | + | inside := domain.Token{ID: "in", Scopes: scopes(domain.ScopeRepoRead), BranchPrefixes: []string{"agent/claude/x/"}} | |
| 642 | + | wide := domain.Token{ID: "wide", Scopes: scopes(domain.ScopeRepoRead), BranchPrefixes: []string{"agent/"}} | |
| 643 | + | sibling := domain.Token{ID: "sib", Scopes: scopes(domain.ScopeRepoRead), BranchPrefixes: []string{"agent/claudex/"}} | |
| 644 | + | free := domain.Token{ID: "free", Scopes: scopes(domain.ScopeRepoRead)} | |
| 645 | + | ||
| 646 | + | f := newTokenFixture(t) | |
| 647 | + | for _, tok := range []domain.Token{wide, sibling, free} { | |
| 648 | + | f.store.EXPECT().Get(ctx, tok.ID).Return(tok, nil).Once() | |
| 649 | + | } | |
| 650 | + | // Out of reach: not found, like an unknown id, and nothing is revoked. | |
| 651 | + | for _, id := range []string{"wide", "sib", "free"} { | |
| 652 | + | assert.ErrorIs(t, f.tokens.Revoke(ctx, creator, id), domain.ErrNotFound, id) | |
| 653 | + | } | |
| 654 | + | // Within its prefixes it revokes, and no web session is signed out. | |
| 655 | + | f.store.EXPECT().Get(ctx, "in").Return(inside, nil).Once() | |
| 656 | + | f.store.EXPECT().Revoke(ctx, "in").Return(nil).Once() | |
| 657 | + | require.NoError(t, f.tokens.Revoke(ctx, creator, "in")) | |
| 658 | + | }) | |
| 659 | + | ||
| 623 | 660 | t.Run("listing hides tokens a limited creator could not have made", func(t *testing.T) { | |
| 624 | 661 | t.Parallel() | |
| 625 | 662 | f := newTokenFixture(t) | |
| @@ -620,6 +620,43 @@ func TestTokensCreateBranchPrefixes(t *testing.T) { | |||||
| 620 | assert.False(t, p.HasOwnerRights()) | 620 | assert.False(t, p.HasOwnerRights()) | ||
| 621 | }) | 621 | }) | ||
| 622 | 622 | ||||
| 623 | + | t.Run("a corrupt empty creator prefix does not cover every child prefix", func(t *testing.T) { | |||
| 624 | + | t.Parallel() | |||
| 625 | + | f := newTokenFixture(t) // the store expects no Create | |||
| 626 | + | corrupt := domain.TokenPrincipal(domain.Token{ | |||
| 627 | + | ID: "c", Scopes: scopes(domain.ScopeTokenAdmin, domain.ScopeRepoWrite), BranchPrefixes: []string{""}, | |||
| 628 | + | }) | |||
| 629 | + | _, err := f.tokens.Create(ctx, corrupt, ports.CreateTokenParams{ | |||
| 630 | + | Label: "x", Scopes: scopes(domain.ScopeRepoWrite), BranchPrefixes: []string{"main/"}, | |||
| 631 | + | }) | |||
| 632 | + | assert.ErrorIs(t, err, domain.ErrForbidden) | |||
| 633 | + | }) | |||
| 634 | + | ||||
| 635 | + | t.Run("revoking is bounded by the prefixes of a limited token", func(t *testing.T) { | |||
| 636 | + | t.Parallel() | |||
| 637 | + | ctx := context.Background() | |||
| 638 | + | creator := domain.TokenPrincipal(domain.Token{ | |||
| 639 | + | ID: "c", Scopes: scopes(domain.ScopeTokenAdmin, domain.ScopeRepoRead), BranchPrefixes: []string{"agent/claude/"}, | |||
| 640 | + | }) | |||
| 641 | + | inside := domain.Token{ID: "in", Scopes: scopes(domain.ScopeRepoRead), BranchPrefixes: []string{"agent/claude/x/"}} | |||
| 642 | + | wide := domain.Token{ID: "wide", Scopes: scopes(domain.ScopeRepoRead), BranchPrefixes: []string{"agent/"}} | |||
| 643 | + | sibling := domain.Token{ID: "sib", Scopes: scopes(domain.ScopeRepoRead), BranchPrefixes: []string{"agent/claudex/"}} | |||
| 644 | + | free := domain.Token{ID: "free", Scopes: scopes(domain.ScopeRepoRead)} | |||
| 645 | + | ||||
| 646 | + | f := newTokenFixture(t) | |||
| 647 | + | for _, tok := range []domain.Token{wide, sibling, free} { | |||
| 648 | + | f.store.EXPECT().Get(ctx, tok.ID).Return(tok, nil).Once() | |||
| 649 | + | } | |||
| 650 | + | // Out of reach: not found, like an unknown id, and nothing is revoked. | |||
| 651 | + | for _, id := range []string{"wide", "sib", "free"} { | |||
| 652 | + | assert.ErrorIs(t, f.tokens.Revoke(ctx, creator, id), domain.ErrNotFound, id) | |||
| 653 | + | } | |||
| 654 | + | // Within its prefixes it revokes, and no web session is signed out. | |||
| 655 | + | f.store.EXPECT().Get(ctx, "in").Return(inside, nil).Once() | |||
| 656 | + | f.store.EXPECT().Revoke(ctx, "in").Return(nil).Once() | |||
| 657 | + | require.NoError(t, f.tokens.Revoke(ctx, creator, "in")) | |||
| 658 | + | }) | |||
| 659 | + | ||||
| 623 | t.Run("listing hides tokens a limited creator could not have made", func(t *testing.T) { | 660 | t.Run("listing hides tokens a limited creator could not have made", func(t *testing.T) { | ||
| 624 | t.Parallel() | 661 | t.Parallel() | ||
| 625 | f := newTokenFixture(t) | 662 | f := newTokenFixture(t) | ||