fix(app): reserve the token labels owner and root
The audit trail shows "root" for the admin token and "owner" for the web owner, so a token labelled that way would pass for them. Creating one is refused as invalid_argument, whatever the case and the surrounding spaces. 'token create' help says so.
- commit
- e2c58e1b8e45164e813d40ed3a3af7100206c4c4
- author
- brice · · 2026-10-10 19:27 UTC
- committer
- brice · · 2026-10-10 19:28 UTC
- parent
- a658df0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +36 -3
M cmd/ermite/api_test.go +2 -0
| @@ -309,6 +309,8 @@ func TestTokenRPCs(t *testing.T) { | |||
| 309 | 309 | "no scope": {Label: "x"}, | |
| 310 | 310 | "unspecified scope": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_UNSPECIFIED}}, | |
| 311 | 311 | "empty label": {Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}}, | |
| 312 | + | "reserved owner": {Label: "owner", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}}, | |
| 313 | + | "reserved root": {Label: " Root ", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}}, | |
| 312 | 314 | "expired": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}, ExpiresAt: timestamppb.New(time.Now().Add(-time.Hour))}, | |
| 313 | 315 | "bad repository": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}, Repos: []string{"Bad Name"}}, | |
| 314 | 316 | } { | |
| @@ -309,6 +309,8 @@ func TestTokenRPCs(t *testing.T) { | |||||
| 309 | "no scope": {Label: "x"}, | 309 | "no scope": {Label: "x"}, | ||
| 310 | "unspecified scope": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_UNSPECIFIED}}, | 310 | "unspecified scope": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_UNSPECIFIED}}, | ||
| 311 | "empty label": {Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}}, | 311 | "empty label": {Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}}, | ||
| 312 | + | "reserved owner": {Label: "owner", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}}, | |||
| 313 | + | "reserved root": {Label: " Root ", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}}, | |||
| 312 | "expired": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}, ExpiresAt: timestamppb.New(time.Now().Add(-time.Hour))}, | 314 | "expired": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}, ExpiresAt: timestamppb.New(time.Now().Add(-time.Hour))}, | ||
| 313 | "bad repository": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}, Repos: []string{"Bad Name"}}, | 315 | "bad repository": {Label: "x", Scopes: []pb.Scope{pb.Scope_SCOPE_REPO_READ}, Repos: []string{"Bad Name"}}, | ||
| 314 | } { | 316 | } { | ||
M cmd/ermite/cli_token.go +3 -1
| @@ -83,7 +83,9 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command { | |||
| 83 | 83 | " agent repo:write, mr:write\n" + | |
| 84 | 84 | " ci repo:read\n" + | |
| 85 | 85 | "A token cannot be more powerful than the one that creates it. --repo limits it to the named\n" + | |
| 86 | - | "repositories (default: all). --expires takes 30d, 2w, 12h or a date; default: never." + jsonTokenHelp, | |
| 86 | + | "repositories (default: all). --expires takes 30d, 2w, 12h or a date; default: never.\n\n" + | |
| 87 | + | "The label is free text of up to 100 bytes, except \"owner\" and \"root\" (in any case): they are\n" + | |
| 88 | + | "what the audit trail shows for the admin token and the web owner, so the server refuses them." + jsonTokenHelp, | |
| 87 | 89 | Args: cobra.ExactArgs(1), | |
| 88 | 90 | } | |
| 89 | 91 | cmd.Flags().StringVar(&preset, "preset", "", "scope preset: "+strings.Join(client.PresetNames, ", ")) | |
| @@ -83,7 +83,9 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command { | |||||
| 83 | " agent repo:write, mr:write\n" + | 83 | " agent repo:write, mr:write\n" + | ||
| 84 | " ci repo:read\n" + | 84 | " ci repo:read\n" + | ||
| 85 | "A token cannot be more powerful than the one that creates it. --repo limits it to the named\n" + | 85 | "A token cannot be more powerful than the one that creates it. --repo limits it to the named\n" + | ||
| 86 | - | "repositories (default: all). --expires takes 30d, 2w, 12h or a date; default: never." + jsonTokenHelp, | 86 | + | "repositories (default: all). --expires takes 30d, 2w, 12h or a date; default: never.\n\n" + |
| 87 | + | "The label is free text of up to 100 bytes, except \"owner\" and \"root\" (in any case): they are\n" + | |||
| 88 | + | "what the audit trail shows for the admin token and the web owner, so the server refuses them." + jsonTokenHelp, | |||
| 87 | Args: cobra.ExactArgs(1), | 89 | Args: cobra.ExactArgs(1), | ||
| 88 | } | 90 | } | ||
| 89 | cmd.Flags().StringVar(&preset, "preset", "", "scope preset: "+strings.Join(client.PresetNames, ", ")) | 91 | cmd.Flags().StringVar(&preset, "preset", "", "scope preset: "+strings.Join(client.PresetNames, ", ")) | ||
M internal/app/tokens.go +14 -1
| @@ -24,6 +24,12 @@ import ( | |||
| 24 | 24 | // MaxTokenLabelLen is the longest token label, in bytes. | |
| 25 | 25 | const MaxTokenLabelLen = 100 | |
| 26 | 26 | ||
| 27 | + | // reservedTokenLabels cannot label a token: they are what the audit trail | |
| 28 | + | // (author of a merge, "created_by" in the log) shows for the admin token and | |
| 29 | + | // for the owner signed in on the web, so a token with one of them would pass | |
| 30 | + | // for the owner. | |
| 31 | + | var reservedTokenLabels = []string{"owner", "root"} | |
| 32 | + | ||
| 27 | 33 | // touchInterval is how often last_used_at of a token is written at most. | |
| 28 | 34 | const touchInterval = time.Minute | |
| 29 | 35 | ||
| @@ -66,7 +72,9 @@ type CreatedToken struct { | |||
| 66 | 72 | // holds, the repositories must lie within its list (a limited token cannot | |
| 67 | 73 | // create an unlimited one) and the expiry must not outlast its own. Violations | |
| 68 | 74 | // give domain.ErrForbidden. Root has no such limits. Invalid settings give | |
| 69 | - | // domain.ErrInvalidToken, an unknown scope domain.ErrInvalidScope. | |
| 75 | + | // domain.ErrInvalidToken (this includes the labels "owner" and "root", which | |
| 76 | + | // are reserved, in any case and around spaces), an unknown scope | |
| 77 | + | // domain.ErrInvalidScope. | |
| 70 | 78 | func (t *Tokens) Create(ctx context.Context, p domain.Principal, params CreateTokenParams) (CreatedToken, error) { | |
| 71 | 79 | if p.IsAnonymous() { | |
| 72 | 80 | return CreatedToken{}, domain.ErrUnauthenticated | |
| @@ -109,6 +117,11 @@ func buildToken(params CreateTokenParams, now time.Time) (domain.Token, error) { | |||
| 109 | 117 | if params.Label == "" || len(params.Label) > MaxTokenLabelLen || !utf8.ValidString(params.Label) { | |
| 110 | 118 | return domain.Token{}, fmt.Errorf("%w: label must be 1 to %d bytes of UTF-8", domain.ErrInvalidToken, MaxTokenLabelLen) | |
| 111 | 119 | } | |
| 120 | + | for _, r := range reservedTokenLabels { | |
| 121 | + | if strings.EqualFold(strings.TrimSpace(params.Label), r) { | |
| 122 | + | return domain.Token{}, fmt.Errorf("%w: label %q is reserved", domain.ErrInvalidToken, r) | |
| 123 | + | } | |
| 124 | + | } | |
| 112 | 125 | if len(params.Scopes) == 0 { | |
| 113 | 126 | return domain.Token{}, fmt.Errorf("%w: at least one scope is required", domain.ErrInvalidToken) | |
| 114 | 127 | } | |
| @@ -24,6 +24,12 @@ import ( | |||||
| 24 | // MaxTokenLabelLen is the longest token label, in bytes. | 24 | // MaxTokenLabelLen is the longest token label, in bytes. | ||
| 25 | const MaxTokenLabelLen = 100 | 25 | const MaxTokenLabelLen = 100 | ||
| 26 | 26 | ||||
| 27 | + | // reservedTokenLabels cannot label a token: they are what the audit trail | |||
| 28 | + | // (author of a merge, "created_by" in the log) shows for the admin token and | |||
| 29 | + | // for the owner signed in on the web, so a token with one of them would pass | |||
| 30 | + | // for the owner. | |||
| 31 | + | var reservedTokenLabels = []string{"owner", "root"} | |||
| 32 | + | ||||
| 27 | // touchInterval is how often last_used_at of a token is written at most. | 33 | // touchInterval is how often last_used_at of a token is written at most. | ||
| 28 | const touchInterval = time.Minute | 34 | const touchInterval = time.Minute | ||
| 29 | 35 | ||||
| @@ -66,7 +72,9 @@ type CreatedToken struct { | |||||
| 66 | // holds, the repositories must lie within its list (a limited token cannot | 72 | // holds, the repositories must lie within its list (a limited token cannot | ||
| 67 | // create an unlimited one) and the expiry must not outlast its own. Violations | 73 | // create an unlimited one) and the expiry must not outlast its own. Violations | ||
| 68 | // give domain.ErrForbidden. Root has no such limits. Invalid settings give | 74 | // give domain.ErrForbidden. Root has no such limits. Invalid settings give | ||
| 69 | - | // domain.ErrInvalidToken, an unknown scope domain.ErrInvalidScope. | 75 | + | // domain.ErrInvalidToken (this includes the labels "owner" and "root", which |
| 76 | + | // are reserved, in any case and around spaces), an unknown scope | |||
| 77 | + | // domain.ErrInvalidScope. | |||
| 70 | func (t *Tokens) Create(ctx context.Context, p domain.Principal, params CreateTokenParams) (CreatedToken, error) { | 78 | func (t *Tokens) Create(ctx context.Context, p domain.Principal, params CreateTokenParams) (CreatedToken, error) { | ||
| 71 | if p.IsAnonymous() { | 79 | if p.IsAnonymous() { | ||
| 72 | return CreatedToken{}, domain.ErrUnauthenticated | 80 | return CreatedToken{}, domain.ErrUnauthenticated | ||
| @@ -109,6 +117,11 @@ func buildToken(params CreateTokenParams, now time.Time) (domain.Token, error) { | |||||
| 109 | if params.Label == "" || len(params.Label) > MaxTokenLabelLen || !utf8.ValidString(params.Label) { | 117 | if params.Label == "" || len(params.Label) > MaxTokenLabelLen || !utf8.ValidString(params.Label) { | ||
| 110 | return domain.Token{}, fmt.Errorf("%w: label must be 1 to %d bytes of UTF-8", domain.ErrInvalidToken, MaxTokenLabelLen) | 118 | return domain.Token{}, fmt.Errorf("%w: label must be 1 to %d bytes of UTF-8", domain.ErrInvalidToken, MaxTokenLabelLen) | ||
| 111 | } | 119 | } | ||
| 120 | + | for _, r := range reservedTokenLabels { | |||
| 121 | + | if strings.EqualFold(strings.TrimSpace(params.Label), r) { | |||
| 122 | + | return domain.Token{}, fmt.Errorf("%w: label %q is reserved", domain.ErrInvalidToken, r) | |||
| 123 | + | } | |||
| 124 | + | } | |||
| 112 | if len(params.Scopes) == 0 { | 125 | if len(params.Scopes) == 0 { | ||
| 113 | return domain.Token{}, fmt.Errorf("%w: at least one scope is required", domain.ErrInvalidToken) | 126 | return domain.Token{}, fmt.Errorf("%w: at least one scope is required", domain.ErrInvalidToken) | ||
| 114 | } | 127 | } | ||
M internal/app/tokens_test.go +17 -1
| @@ -49,6 +49,17 @@ func names(t *testing.T, ss ...string) []domain.RepoName { | |||
| 49 | 49 | return out | |
| 50 | 50 | } | |
| 51 | 51 | ||
| 52 | + | // Labels that only contain a reserved word are fine. | |
| 53 | + | func TestTokensCreateAllowsLabelsNearReserved(t *testing.T) { | |
| 54 | + | ctx := context.Background() | |
| 55 | + | for _, label := range []string{"owner2", "my owner", "roots", "ow ner", "root-ci"} { | |
| 56 | + | f := newTokenFixture(t) | |
| 57 | + | f.store.EXPECT().Create(ctx, mock.Anything).Return(nil) | |
| 58 | + | _, err := f.tokens.Create(ctx, domain.Root(), app.CreateTokenParams{Label: label, Scopes: scopes(domain.ScopeRepoRead)}) | |
| 59 | + | assert.NoError(t, err, label) | |
| 60 | + | } | |
| 61 | + | } | |
| 62 | + | ||
| 52 | 63 | func TestTokensCreate(t *testing.T) { | |
| 53 | 64 | ctx := context.Background() | |
| 54 | 65 | ||
| @@ -116,6 +127,11 @@ func TestTokensCreate(t *testing.T) { | |||
| 116 | 127 | {"empty label", app.CreateTokenParams{Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 117 | 128 | {"long label", app.CreateTokenParams{Label: strings.Repeat("x", 101), Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 118 | 129 | {"bad utf8 label", app.CreateTokenParams{Label: "\xff", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 130 | + | {"reserved owner", app.CreateTokenParams{Label: "owner", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 131 | + | {"reserved root", app.CreateTokenParams{Label: "root", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 132 | + | {"reserved, upper case", app.CreateTokenParams{Label: "OWNER", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 133 | + | {"reserved, mixed case", app.CreateTokenParams{Label: "Root", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 134 | + | {"reserved, padded", app.CreateTokenParams{Label: " \towner \n", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |
| 119 | 135 | {"no scopes", app.CreateTokenParams{Label: "x"}, domain.ErrInvalidToken}, | |
| 120 | 136 | {"unknown scope", app.CreateTokenParams{Label: "x", Scopes: scopes("nope")}, domain.ErrInvalidScope}, | |
| 121 | 137 | {"past expiry", app.CreateTokenParams{Label: "x", Scopes: scopes(domain.ScopeRepoRead), ExpiresAt: now}, domain.ErrInvalidToken}, | |
| @@ -351,7 +367,7 @@ func TestTokensAdminReach(t *testing.T) { | |||
| 351 | 367 | require.NoError(t, err) | |
| 352 | 368 | assert.Equal(t, all, got) | |
| 353 | 369 | f.store.EXPECT().Get(ctx, "x").Return(all[0], nil) | |
| 354 | - | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // root has no limits | |
| 370 | + | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // root has no limits | |
| 355 | 371 | f.sessions.EXPECT().BumpEpoch(ctx).Return(1, nil) // "x" has owner rights | |
| 356 | 372 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "x")) | |
| 357 | 373 | }) | |
| @@ -49,6 +49,17 @@ func names(t *testing.T, ss ...string) []domain.RepoName { | |||||
| 49 | return out | 49 | return out | ||
| 50 | } | 50 | } | ||
| 51 | 51 | ||||
| 52 | + | // Labels that only contain a reserved word are fine. | |||
| 53 | + | func TestTokensCreateAllowsLabelsNearReserved(t *testing.T) { | |||
| 54 | + | ctx := context.Background() | |||
| 55 | + | for _, label := range []string{"owner2", "my owner", "roots", "ow ner", "root-ci"} { | |||
| 56 | + | f := newTokenFixture(t) | |||
| 57 | + | f.store.EXPECT().Create(ctx, mock.Anything).Return(nil) | |||
| 58 | + | _, err := f.tokens.Create(ctx, domain.Root(), app.CreateTokenParams{Label: label, Scopes: scopes(domain.ScopeRepoRead)}) | |||
| 59 | + | assert.NoError(t, err, label) | |||
| 60 | + | } | |||
| 61 | + | } | |||
| 62 | + | ||||
| 52 | func TestTokensCreate(t *testing.T) { | 63 | func TestTokensCreate(t *testing.T) { | ||
| 53 | ctx := context.Background() | 64 | ctx := context.Background() | ||
| 54 | 65 | ||||
| @@ -116,6 +127,11 @@ func TestTokensCreate(t *testing.T) { | |||||
| 116 | {"empty label", app.CreateTokenParams{Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | 127 | {"empty label", app.CreateTokenParams{Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | ||
| 117 | {"long label", app.CreateTokenParams{Label: strings.Repeat("x", 101), Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | 128 | {"long label", app.CreateTokenParams{Label: strings.Repeat("x", 101), Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | ||
| 118 | {"bad utf8 label", app.CreateTokenParams{Label: "\xff", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | 129 | {"bad utf8 label", app.CreateTokenParams{Label: "\xff", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | ||
| 130 | + | {"reserved owner", app.CreateTokenParams{Label: "owner", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |||
| 131 | + | {"reserved root", app.CreateTokenParams{Label: "root", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |||
| 132 | + | {"reserved, upper case", app.CreateTokenParams{Label: "OWNER", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |||
| 133 | + | {"reserved, mixed case", app.CreateTokenParams{Label: "Root", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |||
| 134 | + | {"reserved, padded", app.CreateTokenParams{Label: " \towner \n", Scopes: scopes(domain.ScopeRepoRead)}, domain.ErrInvalidToken}, | |||
| 119 | {"no scopes", app.CreateTokenParams{Label: "x"}, domain.ErrInvalidToken}, | 135 | {"no scopes", app.CreateTokenParams{Label: "x"}, domain.ErrInvalidToken}, | ||
| 120 | {"unknown scope", app.CreateTokenParams{Label: "x", Scopes: scopes("nope")}, domain.ErrInvalidScope}, | 136 | {"unknown scope", app.CreateTokenParams{Label: "x", Scopes: scopes("nope")}, domain.ErrInvalidScope}, | ||
| 121 | {"past expiry", app.CreateTokenParams{Label: "x", Scopes: scopes(domain.ScopeRepoRead), ExpiresAt: now}, domain.ErrInvalidToken}, | 137 | {"past expiry", app.CreateTokenParams{Label: "x", Scopes: scopes(domain.ScopeRepoRead), ExpiresAt: now}, domain.ErrInvalidToken}, | ||
| @@ -351,7 +367,7 @@ func TestTokensAdminReach(t *testing.T) { | |||||
| 351 | require.NoError(t, err) | 367 | require.NoError(t, err) | ||
| 352 | assert.Equal(t, all, got) | 368 | assert.Equal(t, all, got) | ||
| 353 | f.store.EXPECT().Get(ctx, "x").Return(all[0], nil) | 369 | f.store.EXPECT().Get(ctx, "x").Return(all[0], nil) | ||
| 354 | - | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // root has no limits | 370 | + | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // root has no limits |
| 355 | f.sessions.EXPECT().BumpEpoch(ctx).Return(1, nil) // "x" has owner rights | 371 | f.sessions.EXPECT().BumpEpoch(ctx).Return(1, nil) // "x" has owner rights | ||
| 356 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "x")) | 372 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "x")) | ||
| 357 | }) | 373 | }) | ||