fix(cli): check the server supports branch prefixes before creating a token
With --branch-prefix, token create reads the server features first and refuses when token.branch_prefixes is missing or unreadable, so a server that ignores the field never creates an unlimited token. The post-check stays as a backstop.
- commit
- e75174962d080218d03dd8e978c23910b70f61f5
- author
- brice · · 2026-10-12 15:17 UTC
- parent
- 908d505
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +70 -6
M internal/app/cli_token.go +21 -1
| @@ -125,6 +125,11 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command { | |||
| 125 | 125 | if err != nil { | |
| 126 | 126 | return err | |
| 127 | 127 | } | |
| 128 | + | if len(req.GetBranchPrefixes()) > 0 { | |
| 129 | + | if err := requireBranchPrefixes(env.ctx(cmd), c); err != nil { | |
| 130 | + | return err | |
| 131 | + | } | |
| 132 | + | } | |
| 128 | 133 | res, err := c.Tokens().CreateToken(env.ctx(cmd), connect.NewRequest(req)) | |
| 129 | 134 | if err != nil { | |
| 130 | 135 | return c.Describe(err) | |
| @@ -137,7 +142,22 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command { | |||
| 137 | 142 | return cmd | |
| 138 | 143 | } | |
| 139 | 144 | ||
| 140 | - | // checkBranchPrefixes refuses to hand out a token that is less limited than | |
| 145 | + | // requireBranchPrefixes refuses, before anything is created, to ask a server | |
| 146 | + | // for a branch-limited token when it does not list token.branch_prefixes: such | |
| 147 | + | // a server ignores the field. A server whose features cannot be read is refused | |
| 148 | + | // too (fail closed). | |
| 149 | + | func requireBranchPrefixes(ctx context.Context, c *client.Client) error { | |
| 150 | + | res, err := c.Server().GetVersion(ctx, connect.NewRequest(&ermite.GetVersionRequest{})) | |
| 151 | + | if err != nil { | |
| 152 | + | return c.Describe(err) | |
| 153 | + | } | |
| 154 | + | if !slices.Contains(res.Msg.GetFeatures(), client.FeatureTokenBranchPrefixes) { | |
| 155 | + | return fmt.Errorf("the server does not support %s, so it would ignore --branch-prefix and create an unlimited token: nothing was created (upgrade the server, see `ermite version`)", client.FeatureTokenBranchPrefixes) | |
| 156 | + | } | |
| 157 | + | return nil | |
| 158 | + | } | |
| 159 | + | ||
| 160 | + | // checkBranchPrefixes is the backstop of requireBranchPrefixes: it refuses to hand out a token that is less limited than | |
| 141 | 161 | // asked for. A server that predates branch prefixes ignores the field and | |
| 142 | 162 | // creates an unlimited token: the secret is then not printed. | |
| 143 | 163 | func checkBranchPrefixes(req *ermite.CreateTokenRequest, got *ermite.Token) error { | |
| @@ -125,6 +125,11 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command { | |||||
| 125 | if err != nil { | 125 | if err != nil { | ||
| 126 | return err | 126 | return err | ||
| 127 | } | 127 | } | ||
| 128 | + | if len(req.GetBranchPrefixes()) > 0 { | |||
| 129 | + | if err := requireBranchPrefixes(env.ctx(cmd), c); err != nil { | |||
| 130 | + | return err | |||
| 131 | + | } | |||
| 132 | + | } | |||
| 128 | res, err := c.Tokens().CreateToken(env.ctx(cmd), connect.NewRequest(req)) | 133 | res, err := c.Tokens().CreateToken(env.ctx(cmd), connect.NewRequest(req)) | ||
| 129 | if err != nil { | 134 | if err != nil { | ||
| 130 | return c.Describe(err) | 135 | return c.Describe(err) | ||
| @@ -137,7 +142,22 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command { | |||||
| 137 | return cmd | 142 | return cmd | ||
| 138 | } | 143 | } | ||
| 139 | 144 | ||||
| 140 | - | // checkBranchPrefixes refuses to hand out a token that is less limited than | 145 | + | // requireBranchPrefixes refuses, before anything is created, to ask a server |
| 146 | + | // for a branch-limited token when it does not list token.branch_prefixes: such | |||
| 147 | + | // a server ignores the field. A server whose features cannot be read is refused | |||
| 148 | + | // too (fail closed). | |||
| 149 | + | func requireBranchPrefixes(ctx context.Context, c *client.Client) error { | |||
| 150 | + | res, err := c.Server().GetVersion(ctx, connect.NewRequest(&ermite.GetVersionRequest{})) | |||
| 151 | + | if err != nil { | |||
| 152 | + | return c.Describe(err) | |||
| 153 | + | } | |||
| 154 | + | if !slices.Contains(res.Msg.GetFeatures(), client.FeatureTokenBranchPrefixes) { | |||
| 155 | + | return fmt.Errorf("the server does not support %s, so it would ignore --branch-prefix and create an unlimited token: nothing was created (upgrade the server, see `ermite version`)", client.FeatureTokenBranchPrefixes) | |||
| 156 | + | } | |||
| 157 | + | return nil | |||
| 158 | + | } | |||
| 159 | + | ||||
| 160 | + | // checkBranchPrefixes is the backstop of requireBranchPrefixes: it refuses to hand out a token that is less limited than | |||
| 141 | // asked for. A server that predates branch prefixes ignores the field and | 161 | // asked for. A server that predates branch prefixes ignores the field and | ||
| 142 | // creates an unlimited token: the secret is then not printed. | 162 | // creates an unlimited token: the secret is then not printed. | ||
| 143 | func checkBranchPrefixes(req *ermite.CreateTokenRequest, got *ermite.Token) error { | 163 | func checkBranchPrefixes(req *ermite.CreateTokenRequest, got *ermite.Token) error { | ||
M internal/app/cli_token_test.go +49 -5
| @@ -42,6 +42,24 @@ func TestTokenCreateMapsFlags(t *testing.T) { | |||
| 42 | 42 | assert.NotContains(t, res.stderr, "SECRETVALUE") | |
| 43 | 43 | } | |
| 44 | 44 | ||
| 45 | + | // lastCreateToken returns the last CreateToken request the fake received. | |
| 46 | + | func lastCreateToken(t *testing.T, f *fakeAPI) *ermite.CreateTokenRequest { | |
| 47 | + | t.Helper() | |
| 48 | + | calls, reqs := f.snapshot() | |
| 49 | + | for i := len(calls) - 1; i >= 0; i-- { | |
| 50 | + | if calls[i] == "CreateToken" { | |
| 51 | + | return reqs[i].(*ermite.CreateTokenRequest) | |
| 52 | + | } | |
| 53 | + | } | |
| 54 | + | t.Fatal("no CreateToken call") | |
| 55 | + | return nil | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | // withBranchPrefixes is a GetVersion that lists token.branch_prefixes. | |
| 59 | + | func withBranchPrefixes(*ermite.GetVersionRequest) (*ermite.GetVersionResponse, error) { | |
| 60 | + | return &ermite.GetVersionResponse{Version: "v0.4.0", Features: []string{"token.branch_prefixes"}}, nil | |
| 61 | + | } | |
| 62 | + | ||
| 45 | 63 | func TestTokenCreateBranchPrefixes(t *testing.T) { | |
| 46 | 64 | echo := func(r *ermite.CreateTokenRequest) (*ermite.CreateTokenResponse, error) { | |
| 47 | 65 | return &ermite.CreateTokenResponse{ | |
| @@ -54,11 +72,10 @@ func TestTokenCreateBranchPrefixes(t *testing.T) { | |||
| 54 | 72 | } | |
| 55 | 73 | ||
| 56 | 74 | t.Run("maps the repeatable flag, trims and drops duplicates", func(t *testing.T) { | |
| 57 | - | f := &fakeAPI{createToken: echo} | |
| 75 | + | f := &fakeAPI{createToken: echo, getVersion: withBranchPrefixes} | |
| 58 | 76 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "claude", "--preset", "agent", "--repo", "alpha", | |
| 59 | 77 | "--branch-prefix", "agent/claude/", "--branch-prefix", " bot/ ", "--branch-prefix", "agent/claude/").ok(t) | |
| 60 | - | _, reqs := f.snapshot() | |
| 61 | - | assert.Equal(t, []string{"agent/claude/", "bot/"}, reqs[0].(*ermite.CreateTokenRequest).GetBranchPrefixes()) | |
| 78 | + | assert.Equal(t, []string{"agent/claude/", "bot/"}, lastCreateToken(t, f).GetBranchPrefixes()) | |
| 62 | 79 | assert.Equal(t, "ermite_S\n", res.stdout) | |
| 63 | 80 | assert.Contains(t, res.stderr, "branches") | |
| 64 | 81 | assert.Contains(t, res.stderr, "agent/claude/, bot/") | |
| @@ -73,18 +90,45 @@ func TestTokenCreateBranchPrefixes(t *testing.T) { | |||
| 73 | 90 | }) | |
| 74 | 91 | ||
| 75 | 92 | t.Run("json", func(t *testing.T) { | |
| 76 | - | f := &fakeAPI{createToken: echo} | |
| 93 | + | f := &fakeAPI{createToken: echo, getVersion: withBranchPrefixes} | |
| 77 | 94 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "l", "--scope", "repo:write", "--branch-prefix", "a/", "--json").ok(t) | |
| 78 | 95 | assert.JSONEq(t, `{"id":"tok1","label":"l","scopes":["repo:write"],"repos":[],"branch_prefixes":["a/"],"expires_at":null,"created_at":"2026-10-10T12:00:00Z","last_used_at":null,"secret":"ermite_S"}`, res.stdout) | |
| 79 | 96 | }) | |
| 80 | 97 | ||
| 98 | + | t.Run("a server without the feature is asked nothing", func(t *testing.T) { | |
| 99 | + | for name, version := range map[string]func(*ermite.GetVersionRequest) (*ermite.GetVersionResponse, error){ | |
| 100 | + | "feature missing": func(*ermite.GetVersionRequest) (*ermite.GetVersionResponse, error) { | |
| 101 | + | return &ermite.GetVersionResponse{Version: "v0.3.0", Features: []string{"repo.update", "session"}}, nil | |
| 102 | + | }, | |
| 103 | + | "version unreadable": nil, // the fake answers unimplemented | |
| 104 | + | } { | |
| 105 | + | f := &fakeAPI{createToken: echo, getVersion: version} | |
| 106 | + | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "x", "--preset", "agent", "--branch-prefix", "agent/x/") | |
| 107 | + | require.Error(t, res.err, name) | |
| 108 | + | if name == "feature missing" { | |
| 109 | + | assert.Contains(t, res.err.Error(), "token.branch_prefixes", name) | |
| 110 | + | assert.Contains(t, res.err.Error(), "nothing was created", name) | |
| 111 | + | } | |
| 112 | + | calls, _ := f.snapshot() | |
| 113 | + | assert.NotContains(t, calls, "CreateToken", "%s: no token is created", name) | |
| 114 | + | assert.Empty(t, res.stdout, name) | |
| 115 | + | } | |
| 116 | + | }) | |
| 117 | + | ||
| 118 | + | t.Run("without --branch-prefix the features are not read", func(t *testing.T) { | |
| 119 | + | f := &fakeAPI{createToken: echo} // no getVersion: it would answer unimplemented | |
| 120 | + | runCLI(t, envFor(f.serve(t)), "", "token", "create", "x", "--preset", "ci").ok(t) | |
| 121 | + | calls, _ := f.snapshot() | |
| 122 | + | assert.NotContains(t, calls, "GetVersion") | |
| 123 | + | }) | |
| 124 | + | ||
| 81 | 125 | t.Run("a server that ignores the field does not hand out an unlimited token", func(t *testing.T) { | |
| 82 | 126 | f := &fakeAPI{createToken: func(r *ermite.CreateTokenRequest) (*ermite.CreateTokenResponse, error) { | |
| 83 | 127 | return &ermite.CreateTokenResponse{ | |
| 84 | 128 | Token: &ermite.Token{Id: "tok9", Label: r.GetLabel(), Scopes: r.GetScopes(), CreatedAt: ts("2026-10-10T12:00:00Z")}, | |
| 85 | 129 | Secret: "ermite_UNLIMITED", | |
| 86 | 130 | }, nil | |
| 87 | - | }} | |
| 131 | + | }, getVersion: withBranchPrefixes} | |
| 88 | 132 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "x", "--preset", "agent", "--branch-prefix", "agent/x/") | |
| 89 | 133 | require.Error(t, res.err) | |
| 90 | 134 | assert.Contains(t, res.err.Error(), "without the requested branch prefixes") | |
| @@ -42,6 +42,24 @@ func TestTokenCreateMapsFlags(t *testing.T) { | |||||
| 42 | assert.NotContains(t, res.stderr, "SECRETVALUE") | 42 | assert.NotContains(t, res.stderr, "SECRETVALUE") | ||
| 43 | } | 43 | } | ||
| 44 | 44 | ||||
| 45 | + | // lastCreateToken returns the last CreateToken request the fake received. | |||
| 46 | + | func lastCreateToken(t *testing.T, f *fakeAPI) *ermite.CreateTokenRequest { | |||
| 47 | + | t.Helper() | |||
| 48 | + | calls, reqs := f.snapshot() | |||
| 49 | + | for i := len(calls) - 1; i >= 0; i-- { | |||
| 50 | + | if calls[i] == "CreateToken" { | |||
| 51 | + | return reqs[i].(*ermite.CreateTokenRequest) | |||
| 52 | + | } | |||
| 53 | + | } | |||
| 54 | + | t.Fatal("no CreateToken call") | |||
| 55 | + | return nil | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | // withBranchPrefixes is a GetVersion that lists token.branch_prefixes. | |||
| 59 | + | func withBranchPrefixes(*ermite.GetVersionRequest) (*ermite.GetVersionResponse, error) { | |||
| 60 | + | return &ermite.GetVersionResponse{Version: "v0.4.0", Features: []string{"token.branch_prefixes"}}, nil | |||
| 61 | + | } | |||
| 62 | + | ||||
| 45 | func TestTokenCreateBranchPrefixes(t *testing.T) { | 63 | func TestTokenCreateBranchPrefixes(t *testing.T) { | ||
| 46 | echo := func(r *ermite.CreateTokenRequest) (*ermite.CreateTokenResponse, error) { | 64 | echo := func(r *ermite.CreateTokenRequest) (*ermite.CreateTokenResponse, error) { | ||
| 47 | return &ermite.CreateTokenResponse{ | 65 | return &ermite.CreateTokenResponse{ | ||
| @@ -54,11 +72,10 @@ func TestTokenCreateBranchPrefixes(t *testing.T) { | |||||
| 54 | } | 72 | } | ||
| 55 | 73 | ||||
| 56 | t.Run("maps the repeatable flag, trims and drops duplicates", func(t *testing.T) { | 74 | t.Run("maps the repeatable flag, trims and drops duplicates", func(t *testing.T) { | ||
| 57 | - | f := &fakeAPI{createToken: echo} | 75 | + | f := &fakeAPI{createToken: echo, getVersion: withBranchPrefixes} |
| 58 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "claude", "--preset", "agent", "--repo", "alpha", | 76 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "claude", "--preset", "agent", "--repo", "alpha", | ||
| 59 | "--branch-prefix", "agent/claude/", "--branch-prefix", " bot/ ", "--branch-prefix", "agent/claude/").ok(t) | 77 | "--branch-prefix", "agent/claude/", "--branch-prefix", " bot/ ", "--branch-prefix", "agent/claude/").ok(t) | ||
| 60 | - | _, reqs := f.snapshot() | 78 | + | assert.Equal(t, []string{"agent/claude/", "bot/"}, lastCreateToken(t, f).GetBranchPrefixes()) |
| 61 | - | assert.Equal(t, []string{"agent/claude/", "bot/"}, reqs[0].(*ermite.CreateTokenRequest).GetBranchPrefixes()) | |||
| 62 | assert.Equal(t, "ermite_S\n", res.stdout) | 79 | assert.Equal(t, "ermite_S\n", res.stdout) | ||
| 63 | assert.Contains(t, res.stderr, "branches") | 80 | assert.Contains(t, res.stderr, "branches") | ||
| 64 | assert.Contains(t, res.stderr, "agent/claude/, bot/") | 81 | assert.Contains(t, res.stderr, "agent/claude/, bot/") | ||
| @@ -73,18 +90,45 @@ func TestTokenCreateBranchPrefixes(t *testing.T) { | |||||
| 73 | }) | 90 | }) | ||
| 74 | 91 | ||||
| 75 | t.Run("json", func(t *testing.T) { | 92 | t.Run("json", func(t *testing.T) { | ||
| 76 | - | f := &fakeAPI{createToken: echo} | 93 | + | f := &fakeAPI{createToken: echo, getVersion: withBranchPrefixes} |
| 77 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "l", "--scope", "repo:write", "--branch-prefix", "a/", "--json").ok(t) | 94 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "l", "--scope", "repo:write", "--branch-prefix", "a/", "--json").ok(t) | ||
| 78 | assert.JSONEq(t, `{"id":"tok1","label":"l","scopes":["repo:write"],"repos":[],"branch_prefixes":["a/"],"expires_at":null,"created_at":"2026-10-10T12:00:00Z","last_used_at":null,"secret":"ermite_S"}`, res.stdout) | 95 | assert.JSONEq(t, `{"id":"tok1","label":"l","scopes":["repo:write"],"repos":[],"branch_prefixes":["a/"],"expires_at":null,"created_at":"2026-10-10T12:00:00Z","last_used_at":null,"secret":"ermite_S"}`, res.stdout) | ||
| 79 | }) | 96 | }) | ||
| 80 | 97 | ||||
| 98 | + | t.Run("a server without the feature is asked nothing", func(t *testing.T) { | |||
| 99 | + | for name, version := range map[string]func(*ermite.GetVersionRequest) (*ermite.GetVersionResponse, error){ | |||
| 100 | + | "feature missing": func(*ermite.GetVersionRequest) (*ermite.GetVersionResponse, error) { | |||
| 101 | + | return &ermite.GetVersionResponse{Version: "v0.3.0", Features: []string{"repo.update", "session"}}, nil | |||
| 102 | + | }, | |||
| 103 | + | "version unreadable": nil, // the fake answers unimplemented | |||
| 104 | + | } { | |||
| 105 | + | f := &fakeAPI{createToken: echo, getVersion: version} | |||
| 106 | + | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "x", "--preset", "agent", "--branch-prefix", "agent/x/") | |||
| 107 | + | require.Error(t, res.err, name) | |||
| 108 | + | if name == "feature missing" { | |||
| 109 | + | assert.Contains(t, res.err.Error(), "token.branch_prefixes", name) | |||
| 110 | + | assert.Contains(t, res.err.Error(), "nothing was created", name) | |||
| 111 | + | } | |||
| 112 | + | calls, _ := f.snapshot() | |||
| 113 | + | assert.NotContains(t, calls, "CreateToken", "%s: no token is created", name) | |||
| 114 | + | assert.Empty(t, res.stdout, name) | |||
| 115 | + | } | |||
| 116 | + | }) | |||
| 117 | + | ||||
| 118 | + | t.Run("without --branch-prefix the features are not read", func(t *testing.T) { | |||
| 119 | + | f := &fakeAPI{createToken: echo} // no getVersion: it would answer unimplemented | |||
| 120 | + | runCLI(t, envFor(f.serve(t)), "", "token", "create", "x", "--preset", "ci").ok(t) | |||
| 121 | + | calls, _ := f.snapshot() | |||
| 122 | + | assert.NotContains(t, calls, "GetVersion") | |||
| 123 | + | }) | |||
| 124 | + | ||||
| 81 | t.Run("a server that ignores the field does not hand out an unlimited token", func(t *testing.T) { | 125 | t.Run("a server that ignores the field does not hand out an unlimited token", func(t *testing.T) { | ||
| 82 | f := &fakeAPI{createToken: func(r *ermite.CreateTokenRequest) (*ermite.CreateTokenResponse, error) { | 126 | f := &fakeAPI{createToken: func(r *ermite.CreateTokenRequest) (*ermite.CreateTokenResponse, error) { | ||
| 83 | return &ermite.CreateTokenResponse{ | 127 | return &ermite.CreateTokenResponse{ | ||
| 84 | Token: &ermite.Token{Id: "tok9", Label: r.GetLabel(), Scopes: r.GetScopes(), CreatedAt: ts("2026-10-10T12:00:00Z")}, | 128 | Token: &ermite.Token{Id: "tok9", Label: r.GetLabel(), Scopes: r.GetScopes(), CreatedAt: ts("2026-10-10T12:00:00Z")}, | ||
| 85 | Secret: "ermite_UNLIMITED", | 129 | Secret: "ermite_UNLIMITED", | ||
| 86 | }, nil | 130 | }, nil | ||
| 87 | - | }} | 131 | + | }, getVersion: withBranchPrefixes} |
| 88 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "x", "--preset", "agent", "--branch-prefix", "agent/x/") | 132 | res := runCLI(t, envFor(f.serve(t)), "", "token", "create", "x", "--preset", "agent", "--branch-prefix", "agent/x/") | ||
| 89 | require.Error(t, res.err) | 133 | require.Error(t, res.err) | ||
| 90 | assert.Contains(t, res.err.Error(), "without the requested branch prefixes") | 134 | assert.Contains(t, res.err.Error(), "without the requested branch prefixes") | ||