From 7aef76b36e82443bf5ef2f8d268b5f2e5d75cb8d Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 22:25:18 -0500 Subject: [PATCH 01/11] feat(auth): treat pg- delegated grants as project-bound --- cmd/volcano/grant_test.go | 193 +++++++++++++++++++++++++ cmd/volcano/main.go | 45 +++++- docs/authentication.md | 10 ++ docs/scripting.md | 1 + internal/api/grant_errors.go | 28 ++++ internal/api/refusal.go | 18 ++- internal/auth/auth.go | 23 ++- internal/auth/auth_test.go | 15 ++ internal/cmd/connect/connect.go | 2 +- internal/cmd/feedback/feedback.go | 4 +- internal/cmd/feedback/feedback_test.go | 2 +- internal/config/config.go | 65 ++++++++- internal/config/config_test.go | 34 +++++ internal/durable/durable.go | 2 +- internal/lavaflow/lavaflow.go | 2 +- internal/lavaflow/start.go | 2 +- internal/telemetry/send.go | 11 +- internal/telemetry/telemetry.go | 7 +- internal/telemetry/telemetry_test.go | 30 ++++ 19 files changed, 467 insertions(+), 27 deletions(-) create mode 100644 cmd/volcano/grant_test.go create mode 100644 internal/api/grant_errors.go diff --git a/cmd/volcano/grant_test.go b/cmd/volcano/grant_test.go new file mode 100644 index 00000000..262c6704 --- /dev/null +++ b/cmd/volcano/grant_test.go @@ -0,0 +1,193 @@ +package main + +import ( + "bytes" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Kong/volcano-cli/internal/api" + rootcmd "github.com/Kong/volcano-cli/internal/cmd/root" + cliconfig "github.com/Kong/volcano-cli/internal/config" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +// testGrant is a delegated grant as a worker gets it. Every test here checks +// that it never shows up in the output. +const testGrant = cliconfig.DelegatedGrantPrefix + "grant-secret-value" + +// grantRun is one command run the way a Builder worker runs the CLI. +type grantRun struct { + code int + stdout string + stderr string + requests []string + auth []string +} + +// runAsWorker runs the real command tree with the worker's environment: the +// grant in VOLCANO_TOKEN, the API in VOLCANO_API_URL, the project from the +// directory's link file, telemetry off and no prompts. No config loader is +// injected, so the CLI reads its configuration the way it does in the VM. +func runAsWorker(t *testing.T, handler http.HandlerFunc, args ...string) grantRun { + t.Helper() + resetInstructions(t) + api.ResetLastRefusalForTest() + t.Cleanup(api.ResetLastRefusalForTest) + + var mu sync.Mutex + result := grantRun{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + result.requests = append(result.requests, r.Method+" "+r.URL.Path) + result.auth = append(result.auth, r.Header.Get("Authorization")) + mu.Unlock() + handler(w, r) + })) + t.Cleanup(server.Close) + + home := t.TempDir() + workspace := filepath.Join(home, "workspace", "app") + require.NoError(t, os.MkdirAll(filepath.Join(workspace, ".volcano"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(workspace, ".volcano", "project.json"), + []byte(`{"project_id":"`+mainTestProjectAlpha+`"}`), 0o644)) + t.Setenv("HOME", home) + t.Chdir(workspace) + t.Setenv("VOLCANO_TOKEN", testGrant) + t.Setenv("VOLCANO_API_URL", server.URL) + t.Setenv("VOLCANO_PROJECT_ID", "") + t.Setenv("VOLCANO_TELEMETRY_DISABLED", "1") + t.Setenv("VOLCANO_NONINTERACTIVE", "1") + + deps := cliruntime.Deps{HTTPClient: server.Client()} + root := rootcmd.New(deps) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs(args) + result.code = run(root, deps) + result.stdout, result.stderr = stdout.String(), stderr.String() + + assert.NotContains(t, result.stdout, "grant-secret-value", "the grant was printed") + assert.NotContains(t, result.stderr, "grant-secret-value", "the grant was printed") + for _, request := range result.requests { + assert.NotContains(t, request, "/cli/telemetry", "the grant reached telemetry") + assert.NotContains(t, request, "/service-keys", "a grant must never cause a service key") + assert.NotContains(t, request, "/user", "a grant must never reach an account route") + } + return result +} + +// projectFunctionsPath is the path `cloud functions list` reads. +const projectFunctionsPath = "/projects/" + mainTestProjectAlpha + "/functions" + +func TestGrantCallsProjectRoutesWithTheGrant(t *testing.T) { + run := runAsWorker(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == projectFunctionsPath { + respondExitTestJSON(w, http.StatusOK, `{"data":[],"has_more":false,"page":1,"limit":100,"total":0}`) + return + } + respondExitTestJSON(w, http.StatusForbidden, `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`) + }, "cloud", "functions", "list") + + assert.Equal(t, 0, run.code, run.stderr) + assert.Equal(t, []string{"GET " + projectFunctionsPath}, run.requests) + assert.Equal(t, []string{"Bearer " + testGrant}, run.auth) + assert.NotContains(t, run.stderr, "VOLCANO_PROJECT_ID is deprecated", "the link file chose the project") +} + +func TestGrantRefusalsExitCodes(t *testing.T) { + for _, tc := range []struct { + name string + status int + body string + code int + hint string + }{ + { + name: "route not allowed", + status: http.StatusForbidden, + body: `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`, + code: exitPermissionDenied, + hint: "Delegated grants cannot call GET " + projectFunctionsPath + ", so this command does not work with one.", + }, + { + name: "permission missing", + status: http.StatusForbidden, + body: `{"error":"the delegated grant does not carry the permission this endpoint needs","code":"grant_permission_missing"}`, + code: exitPermissionDenied, + hint: "The delegated grant lacks the permission GET " + projectFunctionsPath + " needs.", + }, + { + name: "budget exhausted", + status: http.StatusTooManyRequests, + body: `{"error":"the delegated grant has no mutations left; ask the app for a new grant","code":"grant_budget_exhausted"}`, + code: exitGrantBudgetExhausted, + hint: "HTTP 429: the delegated grant has no mutations left; ask the app for a new grant", + }, + { + // Any other 429, such as the per-grant rate limit, is not the + // budget: retrying after a pause works, a new grant is not needed. + name: "rate limited", + status: http.StatusTooManyRequests, + body: `{"error":"rate limit exceeded","code":"rate_limit_exceeded"}`, + code: exitFailure, + }, + { + name: "expired or revoked", + status: http.StatusUnauthorized, + body: `{"error":"invalid or expired token"}`, + code: exitNotAuthenticated, + hint: "The delegated grant has expired or was revoked. Get a new grant and run the command again.", + }, + } { + t.Run(tc.name, func(t *testing.T) { + run := runAsWorker(t, func(w http.ResponseWriter, _ *http.Request) { + respondExitTestJSON(w, tc.status, tc.body) + }, "cloud", "functions", "list") + + assert.Equal(t, tc.code, run.code, run.stderr) + if tc.hint != "" { + assert.Contains(t, run.stderr, tc.hint) + } + assert.NotContains(t, run.stderr, "only works on the project it was", "not a project mismatch") + }) + } +} + +// A grant was an unknown prefix, so an account token, before the CLI knew it. +// Account commands now refuse it before any request. +func TestGrantIsRefusedOnAccountCommandsBeforeAnyRequest(t *testing.T) { + for _, args := range [][]string{ + {"projects", "list"}, + {"cloud", "functions", "list", "--project", "some-project-name"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + run := runAsWorker(t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("an account call reached the API: %s %s", r.Method, r.URL.Path) + respondExitTestJSON(w, http.StatusForbidden, `{"error":"forbidden"}`) + }, args...) + + assert.Equal(t, exitPermissionDenied, run.code, run.stderr) + assert.Contains(t, run.stderr, "delegated grant (pg-)") + assert.Empty(t, run.requests) + }) + } +} + +// The wrong-project 403 gets the mismatch hint, worded for a grant. +func TestGrantMismatchHintNamesTheGrant(t *testing.T) { + run := runAsWorker(t, func(w http.ResponseWriter, _ *http.Request) { + respondExitTestJSON(w, http.StatusForbidden, `{"error":"project access token is not valid for this project"}`) + }, "cloud", "functions", "list") + + assert.Equal(t, exitPermissionDenied, run.code, run.stderr) + assert.Contains(t, run.stderr, "A delegated grant only works on the project it was granted for") +} diff --git a/cmd/volcano/main.go b/cmd/volcano/main.go index 4ac048ee..50a47697 100644 --- a/cmd/volcano/main.go +++ b/cmd/volcano/main.go @@ -37,6 +37,10 @@ const ( exitNoProject = 3 exitNotFound = 4 exitPermissionDenied = 5 + // exitGrantBudgetExhausted is the API's 429 grant_budget_exhausted: a + // delegated grant used all of its mutations. Its own code, because the + // fix is a new grant and a retry, unlike any other failure. + exitGrantBudgetExhausted = 6 ) func main() { @@ -187,6 +191,8 @@ func printProjectEnvDeprecation(w io.Writer, used bool) { func exitCode(err error) int { status := api.Status(err) switch { + case api.GrantBudgetExhausted(err): + return exitGrantBudgetExhausted case errors.Is(err, config.ErrNotAuthenticated), status == http.StatusUnauthorized: return exitNotAuthenticated case errors.Is(err, config.ErrNoProjectSelected), errors.Is(err, config.ErrNoProjectSelectedForProjectToken): @@ -217,6 +223,40 @@ func printError(w io.Writer, err error, deps cliruntime.Deps) { fmt.Fprintf(w, "Run `%s` to re-authenticate.\n", cliruntime.CommandPath(deps, "login")) } printProjectTokenMismatchHint(w, err, deps) + printDelegatedGrantHint(w, err, deps) +} + +// printDelegatedGrantHint explains a refusal of a pg- delegated grant. The +// API's message says what was refused; this adds which request and what to do. +// It never prints the grant itself. +func printDelegatedGrantHint(w io.Writer, err error, deps cliruntime.Deps) { + status := api.Status(err) + switch { + case status == http.StatusForbidden && api.Code(err) == api.GrantRouteNotAllowedCode: + fmt.Fprintf(w, "Delegated grants cannot call %s, so this command does not work with one.\n", refusedRequest()) + case status == http.StatusForbidden && api.Code(err) == api.GrantPermissionMissingCode: + fmt.Fprintf(w, "The delegated grant lacks the permission %s needs. "+ + "The project owner's consent for the app has to include it.\n", refusedRequest()) + case status == http.StatusUnauthorized && credentialIsDelegatedGrant(deps): + fmt.Fprintln(w, "The delegated grant has expired or was revoked. Get a new grant and run the command again.") + } +} + +// refusedRequest names the most recent refused request, as "POST /projects/…", +// or "this request" when none was recorded. +func refusedRequest() string { + refusal := api.LastRefusal() + if refusal.Method == "" || refusal.Path == "" { + return "this request" + } + return refusal.Method + " " + refusal.Path +} + +// credentialIsDelegatedGrant reports whether the command's credential is a +// delegated grant. +func credentialIsDelegatedGrant(deps cliruntime.Deps) bool { + cfg, err := loadConfig(deps) + return err == nil && config.IsDelegatedGrant(cfg.Token()) } // loadConfig loads the configuration through the runtime's loader, so a @@ -259,7 +299,10 @@ func printProjectTokenMismatchHint(w io.Writer, err error, deps cliruntime.Deps) return } - const scope = "A project access token only works on the project it was created in" + scope := "A project access token only works on the project it was created in" + if refusal.DelegatedGrant { + scope = "A delegated grant only works on the project it was granted for" + } if refusal.ProjectID == selectedProjectID(deps) { fmt.Fprintf(w, "This ran against project %s, the one this CLI is pointed at. %s — check that is the "+ "right one, or run `%s` to switch.\n", diff --git a/docs/authentication.md b/docs/authentication.md index 52586a42..36b6be73 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -104,6 +104,16 @@ selecting a project by name — need an account token, and say so when run with project access token. See [access-tokens.md](access-tokens.md). +### Delegated grants + +A delegated grant (`pg-`) is what a Volcano app, such as the Builder, uses to +act on a project you consented to. It reaches one project, lasts minutes, and +works only for what you consented to, such as deploys, config, variable +writes, and migrations. Pass it in `VOLCANO_TOKEN`; `volcano login --token` +refuses one, because it would outlast the run it was made for. Commands outside +the grant fail with exit code `5`, and a grant that has used up its changes +fails with exit code `6`. See [scripting.md](scripting.md#exit-codes). + Log out at any time: ```bash diff --git a/docs/scripting.md b/docs/scripting.md index 644282b3..6618bf8b 100644 --- a/docs/scripting.md +++ b/docs/scripting.md @@ -16,6 +16,7 @@ failures apart without parsing the error text. | `3` | No project is selected | Run `volcano connect .` to create a project for this directory, or `volcano use ` | | `4` | The API answered HTTP 404, or no project or other cloud resource matched the name or ID | Check the name or ID | | `5` | The API answered HTTP 403, or the command needs an account token | Check the credential's access and the plan's limits | +| `6` | A delegated grant (`pg-`) used all of its changes: the API answered HTTP 429 with code `grant_budget_exhausted` | Get a new grant and run the command again | `volcano sandboxes exec` and `volcano sandboxes shell` return the guest process's exit code instead, and `124` on timeout. See diff --git a/internal/api/grant_errors.go b/internal/api/grant_errors.go new file mode 100644 index 00000000..a8dc7ae9 --- /dev/null +++ b/internal/api/grant_errors.go @@ -0,0 +1,28 @@ +package api + +import ( + "errors" + "net/http" +) + +// Codes the API answers a pg- delegated grant with. Each refusal carries one, +// so a caller can tell them apart without parsing the message. +const ( + // GrantRouteNotAllowedCode is a 403 for a route outside what any delegated + // grant may call. + GrantRouteNotAllowedCode = "grant_route_not_allowed" + // GrantPermissionMissingCode is a 403 for a route the grant may call only + // with a permission it does not carry. + GrantPermissionMissingCode = "grant_permission_missing" + // GrantBudgetExhaustedCode is a 429 for a change made after the grant used + // all of its mutations. The grant still works for reads; a new grant gets a + // new budget. + GrantBudgetExhaustedCode = "grant_budget_exhausted" +) + +// GrantBudgetExhausted reports whether err is the API's 429 for a delegated +// grant with no mutations left. +func GrantBudgetExhausted(err error) bool { + apiErr, ok := errors.AsType[*Error](err) + return ok && apiErr.StatusCode == http.StatusTooManyRequests && apiErr.Code == GrantBudgetExhaustedCode +} diff --git a/internal/api/refusal.go b/internal/api/refusal.go index e3089de7..438e23d6 100644 --- a/internal/api/refusal.go +++ b/internal/api/refusal.go @@ -27,9 +27,17 @@ type Refusal struct { // ProjectID is the project the refused request addressed, empty when its // route was not project-scoped. ProjectID string - // ProjectAccessToken reports whether the refused request carried a pt- - // credential. + // ProjectAccessToken reports whether the refused request carried a + // project-bound credential: a pt- project access token or a pg- delegated + // grant. ProjectAccessToken bool + // DelegatedGrant reports whether the refused request carried a pg- + // delegated grant. + DelegatedGrant bool + // Method and Path are the refused request's method and URL path. The path + // holds IDs and names, never a credential. + Method string + Path string } var ( @@ -54,9 +62,13 @@ func ResetLastRefusalForTest() { } func recordRefusal(req *http.Request) { + token := bearerToken(req.Header.Get("Authorization")) refusal := Refusal{ ProjectID: requestProjectID(req.URL.Path), - ProjectAccessToken: config.IsProjectToken(bearerToken(req.Header.Get("Authorization"))), + ProjectAccessToken: config.IsProjectBound(token), + DelegatedGrant: config.IsDelegatedGrant(token), + Method: req.Method, + Path: req.URL.Path, } refusalMu.Lock() diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 1f49c6b7..eb2e19d1 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -84,10 +84,23 @@ const ( // against and select, by ID or by name; it is optional for an account token // and required for a project access token, which cannot list projects and so // cannot tell the CLI which project it belongs to. +// +// A delegated grant is refused before any request: it lives minutes, so +// saving it to ~/.volcano/config.json would leave a dead credential behind +// that outlasts the run it was minted for. func (s Service) LoginWithToken(ctx context.Context, cfg *config.Config, token, project string) (Credentials, error) { - return s.loginWithToken(ctx, cfg, strings.TrimSpace(token), "", project) + token = strings.TrimSpace(token) + if config.IsDelegatedGrant(token) { + return Credentials{}, ErrDelegatedGrantLogin + } + return s.loginWithToken(ctx, cfg, token, "", project) } +// ErrDelegatedGrantLogin refuses `volcano login --token` with a delegated +// grant. +var ErrDelegatedGrantLogin = errors.New("a delegated grant (" + config.DelegatedGrantPrefix + ") is not saved by login: " + + "set " + config.TokenEnv + " to it for each command instead") + // dashboardDeviceCodeTTL is how long a device code the dashboard started // lives: the API's device grant TTL. The dashboard has already approved it, so // polling ends long before this unless the API is slow to see the approval. @@ -213,7 +226,7 @@ func (s Service) loginWithToken(ctx context.Context, cfg *config.Config, token, } origin := projectNamed - if project == "" && config.IsProjectToken(token) { + if project == "" && config.IsProjectBound(token) { project, origin = unnamedProject(cfg) if project == "" { return Credentials{}, projectTokenNeedsProjectError{} @@ -270,9 +283,9 @@ func resolveLoginProject( return selected, nil } - if config.IsProjectToken(token) { - return nil, fmt.Errorf("invalid project %q: a project access token (%s) cannot look up a project by name, "+ - "so name the project it was minted in by ID", identifier, config.ProjectTokenPrefix) + if config.IsProjectBound(token) { + return nil, fmt.Errorf("invalid project %q: %s cannot look up a project by name, "+ + "so name the project it was minted in by ID", identifier, config.ProjectBoundCredential(token)) } return cliproject.Find(ctx, client, identifier) } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 31c86476..f91c8b4f 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -987,3 +987,18 @@ func TestLoggedInMessageNamesTheAccountWhenKnown(t *testing.T) { assert.Equal(t, "Logged in as account@volcano.dev", LoggedInMessage("account@volcano.dev")) assert.Equal(t, "Logged in successfully", LoggedInMessage("")) } + +// A grant lives minutes; login refuses to save it, before any request, and +// does not echo it. +func TestLoginWithTokenRefusesADelegatedGrant(t *testing.T) { + cfg := testAuthConfig(t) + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + assert.Fail(t, "unexpected request", r.URL.Path) + })) + defer server.Close() + + _, err := NewService(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}). + LoginWithToken(context.Background(), cfg, " pg-grant-secret\n", authAlphaProjectID) + require.ErrorIs(t, err, ErrDelegatedGrantLogin) + assert.NotContains(t, err.Error(), "grant-secret") +} diff --git a/internal/cmd/connect/connect.go b/internal/cmd/connect/connect.go index f5b8b8b1..7cd8c41d 100644 --- a/internal/cmd/connect/connect.go +++ b/internal/cmd/connect/connect.go @@ -386,7 +386,7 @@ func ensureSession(ctx context.Context, opts options, sessions clisession.Factor if token := cfg.Token(); token != "" && !opts.signup && !refused { // A project access token can't be checked without a project; the // project lookup below reports it if it is wrong. - if config.IsProjectToken(token) { + if config.IsProjectBound(token) { return nil } client, err := sessions.APIClient(sessions.APIURL(cfg), token) diff --git a/internal/cmd/feedback/feedback.go b/internal/cmd/feedback/feedback.go index c671b75f..16199791 100644 --- a/internal/cmd/feedback/feedback.go +++ b/internal/cmd/feedback/feedback.go @@ -239,8 +239,8 @@ func printStatus(cmd *cobra.Command) error { fmt.Fprintln(w, "Feedback is enabled, but no report is sent until you log in with `volcano login`.") return nil } - if config.IsProjectToken(cfg.Token()) { - fmt.Fprintln(w, "Feedback is enabled, but a project access token cannot send reports. Log in with `volcano login`.") + if config.IsProjectBound(cfg.Token()) { + fmt.Fprintln(w, "Feedback is enabled, but a project access token or delegated grant cannot send reports. Log in with `volcano login`.") return nil } // A report is sent only once the first-run notice is saved. On a fresh diff --git a/internal/cmd/feedback/feedback_test.go b/internal/cmd/feedback/feedback_test.go index a2610ae6..6b311341 100644 --- a/internal/cmd/feedback/feedback_test.go +++ b/internal/cmd/feedback/feedback_test.go @@ -320,7 +320,7 @@ func TestFeedback_ProjectTokenSendsNothing(t *testing.T) { stdout, _, err = run(t, "status") require.NoError(t, err) - assert.Equal(t, "Feedback is enabled, but a project access token cannot send reports. Log in with `volcano login`.\n", stdout) + assert.Equal(t, "Feedback is enabled, but a project access token or delegated grant cannot send reports. Log in with `volcano login`.\n", stdout) _, stderr, err = run(t, append([]string{"--dry-run"}, reportArgs...)...) require.NoError(t, err) diff --git a/internal/config/config.go b/internal/config/config.go index c8bdd77c..787a687a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -35,6 +35,10 @@ const ( // ProjectTokenPrefix marks a project access token, which reaches only the // project it was minted in. ProjectTokenPrefix = "pt-" + // DelegatedGrantPrefix marks a delegated grant: a short-lived credential a + // first-party app's worker gets for one run on one project, limited to the + // routes and permissions the project's owner consented to. + DelegatedGrantPrefix = "pg-" ) var ( @@ -622,23 +626,49 @@ func (c *Config) RequireProject() error { if strings.TrimSpace(c.ProjectID()) != "" { return nil } + if IsDelegatedGrant(c.Token()) { + return projectBoundError{ + message: "no project selected. A delegated grant (" + DelegatedGrantPrefix + ") is bound to one project: " + + "link this directory to it, or pass --project ", + sentinel: ErrNoProjectSelectedForProjectToken, + } + } if IsProjectToken(c.Token()) { return ErrNoProjectSelectedForProjectToken } return ErrNoProjectSelected } -// RequireAccountToken rejects a project access token on a command that needs -// account-wide access. Any other credential is treated as an account token: the -// server is the authority on what a token may do, and tokens minted before the -// prefixes existed carry neither. +// RequireAccountToken rejects a project-bound credential on a command that +// needs account-wide access. Any other credential is treated as an account +// token: the server is the authority on what a token may do, and tokens minted +// before the prefixes existed carry none. func (c *Config) RequireAccountToken() error { + if IsDelegatedGrant(c.Token()) { + return projectBoundError{ + message: "this command needs an account token (" + AccountTokenPrefix + + ") but the current credential is a delegated grant (" + DelegatedGrantPrefix + + "), which only reaches the project it was granted for", + sentinel: ErrAccountTokenRequired, + } + } if IsProjectToken(c.Token()) { return ErrAccountTokenRequired } return nil } +// projectBoundError is a refusal worded for a delegated grant. It still +// matches the project access token sentinel, so the exit code is the same. +type projectBoundError struct { + message string + sentinel error +} + +func (e projectBoundError) Error() string { return e.message } + +func (e projectBoundError) Is(target error) bool { return target == e.sentinel } + // IsProjectToken reports whether token is a project access token. // // Case-insensitive on purpose. The unknown-prefix default is deliberately @@ -649,6 +679,33 @@ func IsProjectToken(token string) bool { return strings.HasPrefix(strings.ToLower(strings.TrimSpace(token)), ProjectTokenPrefix) } +// IsDelegatedGrant reports whether token is a delegated grant. Case-insensitive +// for the same reason as IsProjectToken. +func IsDelegatedGrant(token string) bool { + return strings.HasPrefix(strings.ToLower(strings.TrimSpace(token)), DelegatedGrantPrefix) +} + +// IsProjectBound reports whether token reaches only one project: a project +// access token or a delegated grant. Neither can list projects, look a project +// up by name, or act for an account. Before pg- was known, a grant fell to the +// unknown-prefix default and was treated as an account token. +func IsProjectBound(token string) bool { + return IsProjectToken(token) || IsDelegatedGrant(token) +} + +// ProjectBoundCredential names a project-bound token's kind for messages, such +// as "a delegated grant (pg-)". It returns "" for any other token. +func ProjectBoundCredential(token string) string { + switch { + case IsDelegatedGrant(token): + return "a delegated grant (" + DelegatedGrantPrefix + ")" + case IsProjectToken(token): + return "a project access token (" + ProjectTokenPrefix + ")" + default: + return "" + } +} + // FirstPartyDeviceClientID resolves the first-party OAuth device client ID. func FirstPartyDeviceClientID() (string, error) { if clientID := strings.TrimSpace(os.Getenv(envFirstPartyDeviceID)); clientID != "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 795cfa07..67e8493c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -514,3 +514,37 @@ func TestIsProjectToken(t *testing.T) { }) } } + +func TestIsDelegatedGrantAndIsProjectBound(t *testing.T) { + for token, want := range map[string][2]bool{ + DelegatedGrantPrefix + "abc": {true, true}, + " PG-abc": {true, true}, + ProjectTokenPrefix + "abc": {false, true}, + AccountTokenPrefix + "abc": {false, false}, + "legacy-token": {false, false}, + "": {false, false}, + } { + t.Run(token, func(t *testing.T) { + assert.Equal(t, want[0], IsDelegatedGrant(token), "IsDelegatedGrant") + assert.Equal(t, want[1], IsProjectBound(token), "IsProjectBound") + }) + } +} + +// A grant was once an unknown prefix, and so an account token. It is refused +// where a pt- token is, with its own words and the same sentinel. +func TestRequireAccountTokenAndProjectRefuseADelegatedGrant(t *testing.T) { + t.Setenv(envToken, "") + t.Setenv(envProjectID, "") + cfg := &Config{UserToken: DelegatedGrantPrefix + "secret-grant"} + + err := cfg.RequireAccountToken() + require.ErrorIs(t, err, ErrAccountTokenRequired) + assert.Contains(t, err.Error(), "delegated grant (pg-)") + assert.NotContains(t, err.Error(), "secret-grant") + + err = cfg.RequireProject() + require.ErrorIs(t, err, ErrNoProjectSelectedForProjectToken) + assert.Contains(t, err.Error(), "delegated grant (pg-)") + assert.NotContains(t, err.Error(), "secret-grant") +} diff --git a/internal/durable/durable.go b/internal/durable/durable.go index 9eae75ae..977cbc31 100644 --- a/internal/durable/durable.go +++ b/internal/durable/durable.go @@ -506,7 +506,7 @@ func (s Service) RequirePerson() error { if err != nil { return err } - if config.IsProjectToken(cfg.Token()) { + if config.IsProjectBound(cfg.Token()) { return errApprovalNeedsAPerson } return nil diff --git a/internal/lavaflow/lavaflow.go b/internal/lavaflow/lavaflow.go index 74fae0cc..974b1e2e 100644 --- a/internal/lavaflow/lavaflow.go +++ b/internal/lavaflow/lavaflow.go @@ -431,7 +431,7 @@ func (s Service) update(ctx context.Context, verb string, update apiclient.Lavaf if err != nil { return nil, err } - if config.IsProjectToken(authenticated.Config.Token()) { + if config.IsProjectBound(authenticated.Config.Token()) { return nil, personDecides(authenticated.Config, config.ErrAccountTokenRequired) } current, err := authenticated.API.GetLavaflowSettings(ctx, authenticated.ProjectID) diff --git a/internal/lavaflow/start.go b/internal/lavaflow/start.go index 3c07cb70..a9bbe274 100644 --- a/internal/lavaflow/start.go +++ b/internal/lavaflow/start.go @@ -61,7 +61,7 @@ func (s Service) StartRun(ctx context.Context, issueID string) (*apiclient.Lavaf if err != nil { return nil, nil, err } - if config.IsProjectToken(authenticated.Config.Token()) { + if config.IsProjectBound(authenticated.Config.Token()) { return nil, nil, personStarts(authenticated.Config, config.ErrAccountTokenRequired) } run, body, err := authenticated.API.CreateLavaflowRun(ctx, authenticated.ProjectID, id) diff --git a/internal/telemetry/send.go b/internal/telemetry/send.go index 56f65ba2..0f50df34 100644 --- a/internal/telemetry/send.go +++ b/internal/telemetry/send.go @@ -52,9 +52,9 @@ func spawnFlush(payload []byte) error { } // Flush reads one event payload from r and posts it to the cloud API's -// /cli/telemetry endpoint, with the saved login token when there is one. The -// server accepts the event without a token. The opt-out is not checked again: -// the process that built the event already did. +// /cli/telemetry endpoint, with the login token when there is one and it is +// an account's. The server accepts the event without a token. The opt-out is +// not checked again: the process that built the event already did. func Flush(ctx context.Context, r io.Reader, client *http.Client) error { payload, err := io.ReadAll(io.LimitReader(r, maxPayloadBytes+1)) if err != nil { @@ -77,7 +77,10 @@ func Flush(ctx context.Context, r io.Reader, client *http.Client) error { } req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", "volcano-cli/"+version.Version) - if token := cfg.Token(); token != "" { + if token := cfg.Token(); token != "" && !config.IsProjectBound(token) { + // A project-bound token names no account, so the server cannot use it + // to attribute the event. Sending it would only put a credential on a + // request that does not need one, from a detached process. req.Header.Set("Authorization", "Bearer "+token) } resp, err := client.Do(req) diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 99e31eed..c9c72c5f 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -239,16 +239,17 @@ func FeedbackDebugByEnv(getenv func(string) string) bool { // LoggedIn reports whether the config holds a login token that can send a // feedback report. `volcano feedback` sends nothing without one. A project -// access token does not count: the server takes reports only from an account. +// access token or delegated grant does not count: the server takes reports +// only from an account. func (r *Recorder) LoggedIn() bool { r.setup() return r.cfg != nil && CanSendFeedback(r.cfg.Token()) } // CanSendFeedback reports whether token can send a feedback report: it is set -// and is not a project access token. +// and is not project-bound. func CanSendFeedback(token string) bool { - return token != "" && !config.IsProjectToken(token) + return token != "" && !config.IsProjectBound(token) } // FeedbackOn reports whether `volcano feedback` may send a report: the config diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go index 9b2911de..4fe2d7a4 100644 --- a/internal/telemetry/telemetry_test.go +++ b/internal/telemetry/telemetry_test.go @@ -814,3 +814,33 @@ func TestAnnounceFeedback_Off(t *testing.T) { assert.False(t, h.recorder().AnnounceFeedback(io.Discard), "feedback follows telemetry") assert.Zero(t, h.store.saves) } + +// A project-bound token names no account, so it never leaves the machine on a +// telemetry request, from the detached flush or anywhere else. +func TestFlush_ProjectBoundTokenSendsNoAuthorization(t *testing.T) { + for _, token := range []string{config.DelegatedGrantPrefix + "grant-secret", config.ProjectTokenPrefix + "token-secret"} { + t.Run(token[:3], func(t *testing.T) { + var gotAuth string + var gotBody []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + gotBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusAccepted) + })) + t.Cleanup(srv.Close) + t.Setenv("HOME", t.TempDir()) + t.Setenv("VOLCANO_API_URL", srv.URL) + t.Setenv("VOLCANO_TOKEN", token) + + require.NoError(t, Flush(t.Context(), strings.NewReader(`{"device_id":"d"}`), srv.Client())) + assert.Empty(t, gotAuth) + assert.NotContains(t, string(gotBody), token) + }) + } +} + +func TestCanSendFeedbackRefusesProjectBoundTokens(t *testing.T) { + assert.True(t, CanSendFeedback(config.AccountTokenPrefix+"abc")) + assert.False(t, CanSendFeedback(config.ProjectTokenPrefix+"abc")) + assert.False(t, CanSendFeedback(config.DelegatedGrantPrefix+"abc")) +} From 0e5ee47028c3fb595bfa959cd0d5dfdd65adcf60 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 22:58:38 -0500 Subject: [PATCH 02/11] docs(auth): describe delegated grants without naming an app --- docs/authentication.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/authentication.md b/docs/authentication.md index 36b6be73..ab180d93 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -106,8 +106,8 @@ project access token. See ### Delegated grants -A delegated grant (`pg-`) is what a Volcano app, such as the Builder, uses to -act on a project you consented to. It reaches one project, lasts minutes, and +A delegated grant (`pg-`) is what a Volcano app uses to act on a project you +consented to. It reaches one project, lasts minutes, and works only for what you consented to, such as deploys, config, variable writes, and migrations. Pass it in `VOLCANO_TOKEN`; `volcano login --token` refuses one, because it would outlast the run it was made for. Commands outside From 5ec9d35c810b4d16d53b8fec5260ac1be98934b6 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 23:19:53 -0500 Subject: [PATCH 03/11] refactor(auth): keep only the grant changes Hosting's refusals require --- cmd/volcano/grant_test.go | 73 +++++++------------------- cmd/volcano/main.go | 42 +-------------- docs/authentication.md | 10 ++-- internal/api/grant_errors.go | 17 ++---- internal/api/refusal.go | 18 ++----- internal/auth/auth.go | 19 ++----- internal/auth/auth_test.go | 15 ------ internal/cmd/feedback/feedback.go | 2 +- internal/cmd/feedback/feedback_test.go | 2 +- internal/config/config.go | 59 ++++----------------- internal/config/config_test.go | 14 ++--- internal/telemetry/send.go | 13 +++-- internal/telemetry/telemetry.go | 5 +- internal/telemetry/telemetry_test.go | 7 ++- 14 files changed, 60 insertions(+), 236 deletions(-) diff --git a/cmd/volcano/grant_test.go b/cmd/volcano/grant_test.go index 262c6704..df5d9e7f 100644 --- a/cmd/volcano/grant_test.go +++ b/cmd/volcano/grant_test.go @@ -103,50 +103,30 @@ func TestGrantCallsProjectRoutesWithTheGrant(t *testing.T) { assert.NotContains(t, run.stderr, "VOLCANO_PROJECT_ID is deprecated", "the link file chose the project") } +// Hosting's refusals keep their exit codes and messages. Only a spent budget +// gets its own code, because a new grant is the fix. func TestGrantRefusalsExitCodes(t *testing.T) { for _, tc := range []struct { name string status int body string code int - hint string + want string }{ - { - name: "route not allowed", - status: http.StatusForbidden, - body: `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`, - code: exitPermissionDenied, - hint: "Delegated grants cannot call GET " + projectFunctionsPath + ", so this command does not work with one.", - }, - { - name: "permission missing", - status: http.StatusForbidden, - body: `{"error":"the delegated grant does not carry the permission this endpoint needs","code":"grant_permission_missing"}`, - code: exitPermissionDenied, - hint: "The delegated grant lacks the permission GET " + projectFunctionsPath + " needs.", - }, - { - name: "budget exhausted", - status: http.StatusTooManyRequests, - body: `{"error":"the delegated grant has no mutations left; ask the app for a new grant","code":"grant_budget_exhausted"}`, - code: exitGrantBudgetExhausted, - hint: "HTTP 429: the delegated grant has no mutations left; ask the app for a new grant", - }, - { - // Any other 429, such as the per-grant rate limit, is not the - // budget: retrying after a pause works, a new grant is not needed. - name: "rate limited", - status: http.StatusTooManyRequests, - body: `{"error":"rate limit exceeded","code":"rate_limit_exceeded"}`, - code: exitFailure, - }, - { - name: "expired or revoked", - status: http.StatusUnauthorized, - body: `{"error":"invalid or expired token"}`, - code: exitNotAuthenticated, - hint: "The delegated grant has expired or was revoked. Get a new grant and run the command again.", - }, + {"route not allowed", http.StatusForbidden, + `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`, + exitPermissionDenied, "delegated grants cannot be used on this endpoint"}, + {"permission missing", http.StatusForbidden, + `{"error":"the delegated grant does not carry the permission this endpoint needs","code":"grant_permission_missing"}`, + exitPermissionDenied, "the delegated grant does not carry the permission this endpoint needs"}, + {"budget exhausted", http.StatusTooManyRequests, + `{"error":"the delegated grant has no mutations left; ask the app for a new grant","code":"grant_budget_exhausted"}`, + exitGrantBudgetExhausted, "the delegated grant has no mutations left; ask the app for a new grant"}, + // Any other 429, such as the per-grant rate limit, is not the budget. + {"rate limited", http.StatusTooManyRequests, + `{"error":"rate limit exceeded","code":"rate_limit_exceeded"}`, exitFailure, "rate limit exceeded"}, + {"expired or revoked", http.StatusUnauthorized, + `{"error":"invalid or expired token"}`, exitNotAuthenticated, "invalid or expired token"}, } { t.Run(tc.name, func(t *testing.T) { run := runAsWorker(t, func(w http.ResponseWriter, _ *http.Request) { @@ -154,16 +134,13 @@ func TestGrantRefusalsExitCodes(t *testing.T) { }, "cloud", "functions", "list") assert.Equal(t, tc.code, run.code, run.stderr) - if tc.hint != "" { - assert.Contains(t, run.stderr, tc.hint) - } - assert.NotContains(t, run.stderr, "only works on the project it was", "not a project mismatch") + assert.Contains(t, run.stderr, tc.want) }) } } // A grant was an unknown prefix, so an account token, before the CLI knew it. -// Account commands now refuse it before any request. +// Account commands now refuse it before any request, as they refuse a pt-. func TestGrantIsRefusedOnAccountCommandsBeforeAnyRequest(t *testing.T) { for _, args := range [][]string{ {"projects", "list"}, @@ -176,18 +153,8 @@ func TestGrantIsRefusedOnAccountCommandsBeforeAnyRequest(t *testing.T) { }, args...) assert.Equal(t, exitPermissionDenied, run.code, run.stderr) - assert.Contains(t, run.stderr, "delegated grant (pg-)") + assert.Contains(t, run.stderr, "this command needs an account token") assert.Empty(t, run.requests) }) } } - -// The wrong-project 403 gets the mismatch hint, worded for a grant. -func TestGrantMismatchHintNamesTheGrant(t *testing.T) { - run := runAsWorker(t, func(w http.ResponseWriter, _ *http.Request) { - respondExitTestJSON(w, http.StatusForbidden, `{"error":"project access token is not valid for this project"}`) - }, "cloud", "functions", "list") - - assert.Equal(t, exitPermissionDenied, run.code, run.stderr) - assert.Contains(t, run.stderr, "A delegated grant only works on the project it was granted for") -} diff --git a/cmd/volcano/main.go b/cmd/volcano/main.go index 50a47697..8f8ef02d 100644 --- a/cmd/volcano/main.go +++ b/cmd/volcano/main.go @@ -38,8 +38,7 @@ const ( exitNotFound = 4 exitPermissionDenied = 5 // exitGrantBudgetExhausted is the API's 429 grant_budget_exhausted: a - // delegated grant used all of its mutations. Its own code, because the - // fix is a new grant and a retry, unlike any other failure. + // delegated grant used all of its mutations, and a new grant is the fix. exitGrantBudgetExhausted = 6 ) @@ -223,40 +222,6 @@ func printError(w io.Writer, err error, deps cliruntime.Deps) { fmt.Fprintf(w, "Run `%s` to re-authenticate.\n", cliruntime.CommandPath(deps, "login")) } printProjectTokenMismatchHint(w, err, deps) - printDelegatedGrantHint(w, err, deps) -} - -// printDelegatedGrantHint explains a refusal of a pg- delegated grant. The -// API's message says what was refused; this adds which request and what to do. -// It never prints the grant itself. -func printDelegatedGrantHint(w io.Writer, err error, deps cliruntime.Deps) { - status := api.Status(err) - switch { - case status == http.StatusForbidden && api.Code(err) == api.GrantRouteNotAllowedCode: - fmt.Fprintf(w, "Delegated grants cannot call %s, so this command does not work with one.\n", refusedRequest()) - case status == http.StatusForbidden && api.Code(err) == api.GrantPermissionMissingCode: - fmt.Fprintf(w, "The delegated grant lacks the permission %s needs. "+ - "The project owner's consent for the app has to include it.\n", refusedRequest()) - case status == http.StatusUnauthorized && credentialIsDelegatedGrant(deps): - fmt.Fprintln(w, "The delegated grant has expired or was revoked. Get a new grant and run the command again.") - } -} - -// refusedRequest names the most recent refused request, as "POST /projects/…", -// or "this request" when none was recorded. -func refusedRequest() string { - refusal := api.LastRefusal() - if refusal.Method == "" || refusal.Path == "" { - return "this request" - } - return refusal.Method + " " + refusal.Path -} - -// credentialIsDelegatedGrant reports whether the command's credential is a -// delegated grant. -func credentialIsDelegatedGrant(deps cliruntime.Deps) bool { - cfg, err := loadConfig(deps) - return err == nil && config.IsDelegatedGrant(cfg.Token()) } // loadConfig loads the configuration through the runtime's loader, so a @@ -299,10 +264,7 @@ func printProjectTokenMismatchHint(w io.Writer, err error, deps cliruntime.Deps) return } - scope := "A project access token only works on the project it was created in" - if refusal.DelegatedGrant { - scope = "A delegated grant only works on the project it was granted for" - } + const scope = "A project access token only works on the project it was created in" if refusal.ProjectID == selectedProjectID(deps) { fmt.Fprintf(w, "This ran against project %s, the one this CLI is pointed at. %s — check that is the "+ "right one, or run `%s` to switch.\n", diff --git a/docs/authentication.md b/docs/authentication.md index ab180d93..7cf9f413 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -107,12 +107,10 @@ project access token. See ### Delegated grants A delegated grant (`pg-`) is what a Volcano app uses to act on a project you -consented to. It reaches one project, lasts minutes, and -works only for what you consented to, such as deploys, config, variable -writes, and migrations. Pass it in `VOLCANO_TOKEN`; `volcano login --token` -refuses one, because it would outlast the run it was made for. Commands outside -the grant fail with exit code `5`, and a grant that has used up its changes -fails with exit code `6`. See [scripting.md](scripting.md#exit-codes). +consented to. Like a project access token, it reaches one project; Volcano +also limits it to the commands you consented to. Pass it in `VOLCANO_TOKEN`. +A grant that has used up its changes fails with exit code `6`. See +[scripting.md](scripting.md#exit-codes). Log out at any time: diff --git a/internal/api/grant_errors.go b/internal/api/grant_errors.go index a8dc7ae9..e8275caa 100644 --- a/internal/api/grant_errors.go +++ b/internal/api/grant_errors.go @@ -5,20 +5,9 @@ import ( "net/http" ) -// Codes the API answers a pg- delegated grant with. Each refusal carries one, -// so a caller can tell them apart without parsing the message. -const ( - // GrantRouteNotAllowedCode is a 403 for a route outside what any delegated - // grant may call. - GrantRouteNotAllowedCode = "grant_route_not_allowed" - // GrantPermissionMissingCode is a 403 for a route the grant may call only - // with a permission it does not carry. - GrantPermissionMissingCode = "grant_permission_missing" - // GrantBudgetExhaustedCode is a 429 for a change made after the grant used - // all of its mutations. The grant still works for reads; a new grant gets a - // new budget. - GrantBudgetExhaustedCode = "grant_budget_exhausted" -) +// GrantBudgetExhaustedCode is the API's 429 code for a change made after a +// delegated grant used all of its mutations. A new grant gets a new budget. +const GrantBudgetExhaustedCode = "grant_budget_exhausted" // GrantBudgetExhausted reports whether err is the API's 429 for a delegated // grant with no mutations left. diff --git a/internal/api/refusal.go b/internal/api/refusal.go index 438e23d6..e3089de7 100644 --- a/internal/api/refusal.go +++ b/internal/api/refusal.go @@ -27,17 +27,9 @@ type Refusal struct { // ProjectID is the project the refused request addressed, empty when its // route was not project-scoped. ProjectID string - // ProjectAccessToken reports whether the refused request carried a - // project-bound credential: a pt- project access token or a pg- delegated - // grant. + // ProjectAccessToken reports whether the refused request carried a pt- + // credential. ProjectAccessToken bool - // DelegatedGrant reports whether the refused request carried a pg- - // delegated grant. - DelegatedGrant bool - // Method and Path are the refused request's method and URL path. The path - // holds IDs and names, never a credential. - Method string - Path string } var ( @@ -62,13 +54,9 @@ func ResetLastRefusalForTest() { } func recordRefusal(req *http.Request) { - token := bearerToken(req.Header.Get("Authorization")) refusal := Refusal{ ProjectID: requestProjectID(req.URL.Path), - ProjectAccessToken: config.IsProjectBound(token), - DelegatedGrant: config.IsDelegatedGrant(token), - Method: req.Method, - Path: req.URL.Path, + ProjectAccessToken: config.IsProjectToken(bearerToken(req.Header.Get("Authorization"))), } refusalMu.Lock() diff --git a/internal/auth/auth.go b/internal/auth/auth.go index eb2e19d1..7764ec66 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -84,23 +84,10 @@ const ( // against and select, by ID or by name; it is optional for an account token // and required for a project access token, which cannot list projects and so // cannot tell the CLI which project it belongs to. -// -// A delegated grant is refused before any request: it lives minutes, so -// saving it to ~/.volcano/config.json would leave a dead credential behind -// that outlasts the run it was minted for. func (s Service) LoginWithToken(ctx context.Context, cfg *config.Config, token, project string) (Credentials, error) { - token = strings.TrimSpace(token) - if config.IsDelegatedGrant(token) { - return Credentials{}, ErrDelegatedGrantLogin - } - return s.loginWithToken(ctx, cfg, token, "", project) + return s.loginWithToken(ctx, cfg, strings.TrimSpace(token), "", project) } -// ErrDelegatedGrantLogin refuses `volcano login --token` with a delegated -// grant. -var ErrDelegatedGrantLogin = errors.New("a delegated grant (" + config.DelegatedGrantPrefix + ") is not saved by login: " + - "set " + config.TokenEnv + " to it for each command instead") - // dashboardDeviceCodeTTL is how long a device code the dashboard started // lives: the API's device grant TTL. The dashboard has already approved it, so // polling ends long before this unless the API is slow to see the approval. @@ -284,8 +271,8 @@ func resolveLoginProject( } if config.IsProjectBound(token) { - return nil, fmt.Errorf("invalid project %q: %s cannot look up a project by name, "+ - "so name the project it was minted in by ID", identifier, config.ProjectBoundCredential(token)) + return nil, fmt.Errorf("invalid project %q: a project access token (%s) cannot look up a project by name, "+ + "so name the project it was minted in by ID", identifier, config.ProjectTokenPrefix) } return cliproject.Find(ctx, client, identifier) } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index f91c8b4f..31c86476 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -987,18 +987,3 @@ func TestLoggedInMessageNamesTheAccountWhenKnown(t *testing.T) { assert.Equal(t, "Logged in as account@volcano.dev", LoggedInMessage("account@volcano.dev")) assert.Equal(t, "Logged in successfully", LoggedInMessage("")) } - -// A grant lives minutes; login refuses to save it, before any request, and -// does not echo it. -func TestLoginWithTokenRefusesADelegatedGrant(t *testing.T) { - cfg := testAuthConfig(t) - server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - assert.Fail(t, "unexpected request", r.URL.Path) - })) - defer server.Close() - - _, err := NewService(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}). - LoginWithToken(context.Background(), cfg, " pg-grant-secret\n", authAlphaProjectID) - require.ErrorIs(t, err, ErrDelegatedGrantLogin) - assert.NotContains(t, err.Error(), "grant-secret") -} diff --git a/internal/cmd/feedback/feedback.go b/internal/cmd/feedback/feedback.go index 16199791..7bab0a4d 100644 --- a/internal/cmd/feedback/feedback.go +++ b/internal/cmd/feedback/feedback.go @@ -240,7 +240,7 @@ func printStatus(cmd *cobra.Command) error { return nil } if config.IsProjectBound(cfg.Token()) { - fmt.Fprintln(w, "Feedback is enabled, but a project access token or delegated grant cannot send reports. Log in with `volcano login`.") + fmt.Fprintln(w, "Feedback is enabled, but a project access token cannot send reports. Log in with `volcano login`.") return nil } // A report is sent only once the first-run notice is saved. On a fresh diff --git a/internal/cmd/feedback/feedback_test.go b/internal/cmd/feedback/feedback_test.go index 6b311341..a2610ae6 100644 --- a/internal/cmd/feedback/feedback_test.go +++ b/internal/cmd/feedback/feedback_test.go @@ -320,7 +320,7 @@ func TestFeedback_ProjectTokenSendsNothing(t *testing.T) { stdout, _, err = run(t, "status") require.NoError(t, err) - assert.Equal(t, "Feedback is enabled, but a project access token or delegated grant cannot send reports. Log in with `volcano login`.\n", stdout) + assert.Equal(t, "Feedback is enabled, but a project access token cannot send reports. Log in with `volcano login`.\n", stdout) _, stderr, err = run(t, append([]string{"--dry-run"}, reportArgs...)...) require.NoError(t, err) diff --git a/internal/config/config.go b/internal/config/config.go index 787a687a..e1234493 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -36,8 +36,7 @@ const ( // project it was minted in. ProjectTokenPrefix = "pt-" // DelegatedGrantPrefix marks a delegated grant: a short-lived credential a - // first-party app's worker gets for one run on one project, limited to the - // routes and permissions the project's owner consented to. + // first-party app's worker gets for one run on one project. DelegatedGrantPrefix = "pg-" ) @@ -626,49 +625,23 @@ func (c *Config) RequireProject() error { if strings.TrimSpace(c.ProjectID()) != "" { return nil } - if IsDelegatedGrant(c.Token()) { - return projectBoundError{ - message: "no project selected. A delegated grant (" + DelegatedGrantPrefix + ") is bound to one project: " + - "link this directory to it, or pass --project ", - sentinel: ErrNoProjectSelectedForProjectToken, - } - } - if IsProjectToken(c.Token()) { + if IsProjectBound(c.Token()) { return ErrNoProjectSelectedForProjectToken } return ErrNoProjectSelected } -// RequireAccountToken rejects a project-bound credential on a command that -// needs account-wide access. Any other credential is treated as an account -// token: the server is the authority on what a token may do, and tokens minted -// before the prefixes existed carry none. +// RequireAccountToken rejects a project access token on a command that needs +// account-wide access. Any other credential is treated as an account token: the +// server is the authority on what a token may do, and tokens minted before the +// prefixes existed carry neither. func (c *Config) RequireAccountToken() error { - if IsDelegatedGrant(c.Token()) { - return projectBoundError{ - message: "this command needs an account token (" + AccountTokenPrefix + - ") but the current credential is a delegated grant (" + DelegatedGrantPrefix + - "), which only reaches the project it was granted for", - sentinel: ErrAccountTokenRequired, - } - } - if IsProjectToken(c.Token()) { + if IsProjectBound(c.Token()) { return ErrAccountTokenRequired } return nil } -// projectBoundError is a refusal worded for a delegated grant. It still -// matches the project access token sentinel, so the exit code is the same. -type projectBoundError struct { - message string - sentinel error -} - -func (e projectBoundError) Error() string { return e.message } - -func (e projectBoundError) Is(target error) bool { return target == e.sentinel } - // IsProjectToken reports whether token is a project access token. // // Case-insensitive on purpose. The unknown-prefix default is deliberately @@ -686,26 +659,12 @@ func IsDelegatedGrant(token string) bool { } // IsProjectBound reports whether token reaches only one project: a project -// access token or a delegated grant. Neither can list projects, look a project -// up by name, or act for an account. Before pg- was known, a grant fell to the -// unknown-prefix default and was treated as an account token. +// access token or a delegated grant. Without it, a grant would fall to the +// unknown-prefix default and be treated as an account token. func IsProjectBound(token string) bool { return IsProjectToken(token) || IsDelegatedGrant(token) } -// ProjectBoundCredential names a project-bound token's kind for messages, such -// as "a delegated grant (pg-)". It returns "" for any other token. -func ProjectBoundCredential(token string) string { - switch { - case IsDelegatedGrant(token): - return "a delegated grant (" + DelegatedGrantPrefix + ")" - case IsProjectToken(token): - return "a project access token (" + ProjectTokenPrefix + ")" - default: - return "" - } -} - // FirstPartyDeviceClientID resolves the first-party OAuth device client ID. func FirstPartyDeviceClientID() (string, error) { if clientID := strings.TrimSpace(os.Getenv(envFirstPartyDeviceID)); clientID != "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 67e8493c..ee9b7c21 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -532,19 +532,11 @@ func TestIsDelegatedGrantAndIsProjectBound(t *testing.T) { } // A grant was once an unknown prefix, and so an account token. It is refused -// where a pt- token is, with its own words and the same sentinel. +// where a pt- token is, with the same sentinels. func TestRequireAccountTokenAndProjectRefuseADelegatedGrant(t *testing.T) { t.Setenv(envToken, "") t.Setenv(envProjectID, "") cfg := &Config{UserToken: DelegatedGrantPrefix + "secret-grant"} - - err := cfg.RequireAccountToken() - require.ErrorIs(t, err, ErrAccountTokenRequired) - assert.Contains(t, err.Error(), "delegated grant (pg-)") - assert.NotContains(t, err.Error(), "secret-grant") - - err = cfg.RequireProject() - require.ErrorIs(t, err, ErrNoProjectSelectedForProjectToken) - assert.Contains(t, err.Error(), "delegated grant (pg-)") - assert.NotContains(t, err.Error(), "secret-grant") + require.ErrorIs(t, cfg.RequireAccountToken(), ErrAccountTokenRequired) + require.ErrorIs(t, cfg.RequireProject(), ErrNoProjectSelectedForProjectToken) } diff --git a/internal/telemetry/send.go b/internal/telemetry/send.go index 0f50df34..5aacdeff 100644 --- a/internal/telemetry/send.go +++ b/internal/telemetry/send.go @@ -52,9 +52,9 @@ func spawnFlush(payload []byte) error { } // Flush reads one event payload from r and posts it to the cloud API's -// /cli/telemetry endpoint, with the login token when there is one and it is -// an account's. The server accepts the event without a token. The opt-out is -// not checked again: the process that built the event already did. +// /cli/telemetry endpoint, with the saved login token when there is one. The +// server accepts the event without a token. The opt-out is not checked again: +// the process that built the event already did. func Flush(ctx context.Context, r io.Reader, client *http.Client) error { payload, err := io.ReadAll(io.LimitReader(r, maxPayloadBytes+1)) if err != nil { @@ -77,10 +77,9 @@ func Flush(ctx context.Context, r io.Reader, client *http.Client) error { } req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", "volcano-cli/"+version.Version) - if token := cfg.Token(); token != "" && !config.IsProjectBound(token) { - // A project-bound token names no account, so the server cannot use it - // to attribute the event. Sending it would only put a credential on a - // request that does not need one, from a detached process. + // A delegated grant is never sent: the server ignores it here, and the + // flush is a detached process the grant's run does not control. + if token := cfg.Token(); token != "" && !config.IsDelegatedGrant(token) { req.Header.Set("Authorization", "Bearer "+token) } resp, err := client.Do(req) diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index c9c72c5f..62a5beb4 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -239,15 +239,14 @@ func FeedbackDebugByEnv(getenv func(string) string) bool { // LoggedIn reports whether the config holds a login token that can send a // feedback report. `volcano feedback` sends nothing without one. A project -// access token or delegated grant does not count: the server takes reports -// only from an account. +// access token does not count: the server takes reports only from an account. func (r *Recorder) LoggedIn() bool { r.setup() return r.cfg != nil && CanSendFeedback(r.cfg.Token()) } // CanSendFeedback reports whether token can send a feedback report: it is set -// and is not project-bound. +// and is not a project access token. func CanSendFeedback(token string) bool { return token != "" && !config.IsProjectBound(token) } diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go index 4fe2d7a4..cd3060cf 100644 --- a/internal/telemetry/telemetry_test.go +++ b/internal/telemetry/telemetry_test.go @@ -815,10 +815,9 @@ func TestAnnounceFeedback_Off(t *testing.T) { assert.Zero(t, h.store.saves) } -// A project-bound token names no account, so it never leaves the machine on a -// telemetry request, from the detached flush or anywhere else. -func TestFlush_ProjectBoundTokenSendsNoAuthorization(t *testing.T) { - for _, token := range []string{config.DelegatedGrantPrefix + "grant-secret", config.ProjectTokenPrefix + "token-secret"} { +// A delegated grant never leaves the machine on a telemetry request. +func TestFlush_DelegatedGrantSendsNoAuthorization(t *testing.T) { + for _, token := range []string{config.DelegatedGrantPrefix + "grant-secret"} { t.Run(token[:3], func(t *testing.T) { var gotAuth string var gotBody []byte From 82f35eaab11c7ad79fa650ac2868050be0cd924f Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 23:26:40 -0500 Subject: [PATCH 04/11] style(auth): format the grant tests --- cmd/volcano/grant_test.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/cmd/volcano/grant_test.go b/cmd/volcano/grant_test.go index df5d9e7f..f6d13ad1 100644 --- a/cmd/volcano/grant_test.go +++ b/cmd/volcano/grant_test.go @@ -113,20 +113,30 @@ func TestGrantRefusalsExitCodes(t *testing.T) { code int want string }{ - {"route not allowed", http.StatusForbidden, + { + "route not allowed", http.StatusForbidden, `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`, - exitPermissionDenied, "delegated grants cannot be used on this endpoint"}, - {"permission missing", http.StatusForbidden, + exitPermissionDenied, "delegated grants cannot be used on this endpoint", + }, + { + "permission missing", http.StatusForbidden, `{"error":"the delegated grant does not carry the permission this endpoint needs","code":"grant_permission_missing"}`, - exitPermissionDenied, "the delegated grant does not carry the permission this endpoint needs"}, - {"budget exhausted", http.StatusTooManyRequests, + exitPermissionDenied, "the delegated grant does not carry the permission this endpoint needs", + }, + { + "budget exhausted", http.StatusTooManyRequests, `{"error":"the delegated grant has no mutations left; ask the app for a new grant","code":"grant_budget_exhausted"}`, - exitGrantBudgetExhausted, "the delegated grant has no mutations left; ask the app for a new grant"}, + exitGrantBudgetExhausted, "the delegated grant has no mutations left; ask the app for a new grant", + }, // Any other 429, such as the per-grant rate limit, is not the budget. - {"rate limited", http.StatusTooManyRequests, - `{"error":"rate limit exceeded","code":"rate_limit_exceeded"}`, exitFailure, "rate limit exceeded"}, - {"expired or revoked", http.StatusUnauthorized, - `{"error":"invalid or expired token"}`, exitNotAuthenticated, "invalid or expired token"}, + { + "rate limited", http.StatusTooManyRequests, + `{"error":"rate limit exceeded","code":"rate_limit_exceeded"}`, exitFailure, "rate limit exceeded", + }, + { + "expired or revoked", http.StatusUnauthorized, + `{"error":"invalid or expired token"}`, exitNotAuthenticated, "invalid or expired token", + }, } { t.Run(tc.name, func(t *testing.T) { run := runAsWorker(t, func(w http.ResponseWriter, _ *http.Request) { From d1ab0dfa1e5114d101042463db779d490ed593a5 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Thu, 12 Nov 2026 17:15:51 -0500 Subject: [PATCH 05/11] fix(auth): refuse to log in with a delegated grant --- docs/authentication.md | 1 + internal/auth/auth.go | 8 ++++++++ internal/cmd/auth/auth_test.go | 22 ++++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/docs/authentication.md b/docs/authentication.md index 7cf9f413..6e51fa07 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -109,6 +109,7 @@ project access token. See A delegated grant (`pg-`) is what a Volcano app uses to act on a project you consented to. Like a project access token, it reaches one project; Volcano also limits it to the commands you consented to. Pass it in `VOLCANO_TOKEN`. +`volcano login` refuses a grant, so it is never saved to disk. A grant that has used up its changes fails with exit code `6`. See [scripting.md](scripting.md#exit-codes). diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 7764ec66..48aebf22 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -201,10 +201,18 @@ func (projectTokenNeedsProjectError) Is(target error) bool { return target == config.ErrNoProjectSelectedForProjectToken } +// ErrDelegatedGrantLogin refuses `login --token` with a delegated grant. +var ErrDelegatedGrantLogin = errors.New("login does not save a delegated grant (" + config.DelegatedGrantPrefix + + "): pass it to each command in VOLCANO_TOKEN instead") + // loginWithToken validates token, an account or project access token, for // LoginWithToken and LoginWithDeviceCode. userID is the user a device login // named, if any. func (s Service) loginWithToken(ctx context.Context, cfg *config.Config, token, userID, project string) (Credentials, error) { + // A grant is never saved to disk, so it is refused before any request. + if config.IsDelegatedGrant(token) { + return Credentials{}, ErrDelegatedGrantLogin + } project = strings.TrimSpace(project) apiURL := s.apiURL(cfg) client, err := s.sessions.APIClient(apiURL, token) diff --git a/internal/cmd/auth/auth_test.go b/internal/cmd/auth/auth_test.go index 341b354e..6f2b81f5 100644 --- a/internal/cmd/auth/auth_test.go +++ b/internal/cmd/auth/auth_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "github.com/Kong/volcano-cli/internal/api" + cliauth "github.com/Kong/volcano-cli/internal/auth" cliconfig "github.com/Kong/volcano-cli/internal/config" cliruntime "github.com/Kong/volcano-cli/internal/runtime" ) @@ -222,6 +223,27 @@ func TestLoginRefusesATokenAndADeviceCodeTogether(t *testing.T) { require.ErrorContains(t, err, "none of the others can be") } +// A delegated grant is never saved: login refuses it before any request. +func TestLoginRefusesADelegatedGrant(t *testing.T) { + setAuthTestHome(t) + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + + for _, token := range []string{cliconfig.DelegatedGrantPrefix + "grant-secret", " PG-grant-secret\n"} { + out, err := executeAuthCommand(t, NewLogin(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}), + "--token", token, "--project", authProjectID) + require.ErrorIs(t, err, cliauth.ErrDelegatedGrantLogin) + require.ErrorContains(t, err, "pass it to each command in VOLCANO_TOKEN") + assert.NotContains(t, err.Error()+out, "grant-secret") + } + + path, err := cliconfig.Path() + require.NoError(t, err) + assert.NoFileExists(t, path, "nothing is saved") +} + // Nothing can discover which project a pt- token belongs to: listing projects // is exactly what it cannot do. Saying so beats saving a credential that then // fails on every command. From b2efdbc42b43a555414556f992d6312259c196e2 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 08:06:54 -0500 Subject: [PATCH 06/11] fix(telemetry): redact delegated grants in error text, resources and feedback --- docs/telemetry.md | 2 +- internal/telemetry/client_test.go | 6 ++++++ internal/telemetry/fields.go | 2 +- internal/telemetry/fields_test.go | 12 +++++++----- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/telemetry.md b/docs/telemetry.md index c1fb411a..b3388fb4 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -39,7 +39,7 @@ When a command fails, the event also carries the type of failure (`api`, `network`, `local_not_running`, `usage`, or `other`), the HTTP status and error code of an API error, and the error message. Before sending the message, the CLI replaces your home directory with `~`, strips terminal color codes, and -removes bearer tokens, JWTs, Volcano keys and tokens (`pk-`, `pt-`, `sk-`, +removes bearer tokens, JWTs, Volcano keys and tokens (`pk-`, `pt-`, `pg-`, `sk-`, `ak-`, `vbt_`, `vpg_`, `vcb_`, `sbp1.`), passwords in URLs, email addresses, and URL query strings. It then cuts the message to 500 characters. diff --git a/internal/telemetry/client_test.go b/internal/telemetry/client_test.go index a8fa7d3b..36f56057 100644 --- a/internal/telemetry/client_test.go +++ b/internal/telemetry/client_test.go @@ -125,6 +125,12 @@ func TestScrubText(t *testing.T) { assert.Equal(t, want, ScrubText(in, "/home/dev")) } +// A delegated grant pasted into a feedback report is removed like any other +// Volcano token. +func TestScrubText_DelegatedGrant(t *testing.T) { + assert.Equal(t, "VOLCANO_TOKEN=[redacted] failed", ScrubText("VOLCANO_TOKEN=pg-0123456789abcdefABCD failed", "")) +} + func TestScrubText_BearerAcrossLines(t *testing.T) { assert.Equal(t, "Authorization: Bearer [redacted]\nnext", ScrubText("Authorization: Bearer\nabc.def\nnext", "")) } diff --git a/internal/telemetry/fields.go b/internal/telemetry/fields.go index a9355ade..c6d8fb0d 100644 --- a/internal/telemetry/fields.go +++ b/internal/telemetry/fields.go @@ -177,7 +177,7 @@ const redacted = "[redacted]" var ( bearerPattern = regexp.MustCompile(`(?i)\bbearer\s+\S+`) jwtPattern = regexp.MustCompile(`eyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]*`) - keyPattern = regexp.MustCompile(`\b(?:(?:(?:pk|pt|sk|ak)-|vbt_|vpg_|vcb_)[A-Za-z0-9_-]{16,}|sbp1\.[A-Za-z0-9+/=:]{16,})`) + keyPattern = regexp.MustCompile(`\b(?:(?:(?:pk|pt|pg|sk|ak)-|vbt_|vpg_|vcb_)[A-Za-z0-9_-]{16,}|sbp1\.[A-Za-z0-9+/=:]{16,})`) userPattern = regexp.MustCompile(`([a-zA-Z][a-zA-Z0-9+.-]*://)[^\s/?#]*@`) emailPattern = regexp.MustCompile(`[A-Za-z0-9._%+-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}`) queryPattern = regexp.MustCompile(`(https?://[^\s?#]+)\?[^\s#]*`) diff --git a/internal/telemetry/fields_test.go b/internal/telemetry/fields_test.go index a5706e6a..b87ca65a 100644 --- a/internal/telemetry/fields_test.go +++ b/internal/telemetry/fields_test.go @@ -49,6 +49,7 @@ func TestResources(t *testing.T) { {"positional wins over flag", "get ", []string{"a", "--function", "b"}, map[string]string{"function": "a"}}, {"slice flag value is dropped", "list", []string{"--database", "a,b"}, nil}, {"secret-shaped value is dropped", "get ", []string{"pk-abcdefghijklmnopqrstuvwx"}, nil}, + {"grant-shaped value is dropped", "get ", []string{"pg-abcdefghijklmnopqrstuvwx"}, nil}, {"odd characters are dropped", "get ", []string{"a;rm -rf"}, nil}, } for _, tc := range cases { @@ -98,11 +99,12 @@ func TestResources_Capped(t *testing.T) { func TestRedact(t *testing.T) { jwt := "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxIn0.c2lnbmF0dXJl" cases := map[string]string{ - "Authorization: Bearer abc.def-ghi": "Authorization: Bearer [redacted]", - "token " + jwt + " expired": "token [redacted] expired", - "key pk-0123456789abcdefABCD rejected": "key [redacted] rejected", - "used pt-0123456789abcdefABCD, sk-0123456789abcdefABCD": "used [redacted], [redacted]", - "pk-short": "pk-short", + "Authorization: Bearer abc.def-ghi": "Authorization: Bearer [redacted]", + "token " + jwt + " expired": "token [redacted] expired", + "key pk-0123456789abcdefABCD rejected": "key [redacted] rejected", + "used pt-0123456789abcdefABCD, sk-0123456789abcdefABCD": "used [redacted], [redacted]", + "grant pg-0123456789abcdefABCD is spent": "grant [redacted] is spent", + "pk-short": "pk-short", "proxy sbp1.v2:0123456789abcdef:AbC+dEf/GhI=jKl= denied": "proxy [redacted] denied", "proxy sbp1.AbC+dEf/GhI0123456789= denied": "proxy [redacted] denied", "exchange vbt_0123456789abcdefABCD failed": "exchange [redacted] failed", From d5f920888e8f571e1fa2b55dd828a579f44cfe38 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 08:06:55 -0500 Subject: [PATCH 07/11] docs(auth): list grants with the other credentials and give logout its own heading --- docs/authentication.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/authentication.md b/docs/authentication.md index 6e51fa07..aceba073 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -52,14 +52,17 @@ export VOLCANO_TOKEN=pk-xxxxxxxxxx ## Account tokens and project access tokens -The CLI accepts two kinds of credential: +The CLI accepts three kinds of credential: | Credential | Reaches | Get one with | |---|---|---| | Account token (`pk-`) | Every project you own | `volcano login` | | Project access token (`pt-`) | One project | `volcano cloud access-tokens create` | +| Delegated grant (`pg-`) | One project, only the commands you consented to | An app you consent to | -Both work with `volcano login --token` and with `VOLCANO_TOKEN`. +Account tokens and project access tokens work with `volcano login --token` and +with `VOLCANO_TOKEN`. A delegated grant works only in `VOLCANO_TOKEN`; see +[Delegated grants](#delegated-grants). The prompt on the dashboard's Projects page holds a device code instead of a token. It works with `--device-code` on `volcano login` or `volcano connect`: @@ -113,6 +116,8 @@ also limits it to the commands you consented to. Pass it in `VOLCANO_TOKEN`. A grant that has used up its changes fails with exit code `6`. See [scripting.md](scripting.md#exit-codes). +## Log out + Log out at any time: ```bash From 94e843a7a7ae6ff6c864024ca79c15f15bb54fef Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 08:23:15 -0500 Subject: [PATCH 08/11] docs(telemetry): say feedback refuses delegated grants as well as project access tokens --- internal/telemetry/telemetry.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 62a5beb4..3f7268ef 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -238,15 +238,16 @@ func FeedbackDebugByEnv(getenv func(string) string) bool { } // LoggedIn reports whether the config holds a login token that can send a -// feedback report. `volcano feedback` sends nothing without one. A project -// access token does not count: the server takes reports only from an account. +// feedback report. `volcano feedback` sends nothing without one. A credential +// tied to one project, a project access token or a delegated grant, does not +// count: the server takes reports only from an account. func (r *Recorder) LoggedIn() bool { r.setup() return r.cfg != nil && CanSendFeedback(r.cfg.Token()) } // CanSendFeedback reports whether token can send a feedback report: it is set -// and is not a project access token. +// and is not tied to one project (a project access token or a delegated grant). func CanSendFeedback(token string) bool { return token != "" && !config.IsProjectBound(token) } From ea51c75ad5ec7b656b3510be8b38ed2d0ddeff59 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 08:33:01 -0500 Subject: [PATCH 09/11] revert(auth): leave volcano login unchanged for delegated grants --- docs/authentication.md | 3 +-- internal/auth/auth.go | 12 ++---------- internal/cmd/auth/auth_test.go | 22 ---------------------- 3 files changed, 3 insertions(+), 34 deletions(-) diff --git a/docs/authentication.md b/docs/authentication.md index aceba073..1d9e0601 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -111,8 +111,7 @@ project access token. See A delegated grant (`pg-`) is what a Volcano app uses to act on a project you consented to. Like a project access token, it reaches one project; Volcano -also limits it to the commands you consented to. Pass it in `VOLCANO_TOKEN`. -`volcano login` refuses a grant, so it is never saved to disk. +also limits it to the commands you consented to. Pass it in `VOLCANO_TOKEN`; don't save it with `volcano login`. A grant that has used up its changes fails with exit code `6`. See [scripting.md](scripting.md#exit-codes). diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 48aebf22..1f49c6b7 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -201,18 +201,10 @@ func (projectTokenNeedsProjectError) Is(target error) bool { return target == config.ErrNoProjectSelectedForProjectToken } -// ErrDelegatedGrantLogin refuses `login --token` with a delegated grant. -var ErrDelegatedGrantLogin = errors.New("login does not save a delegated grant (" + config.DelegatedGrantPrefix + - "): pass it to each command in VOLCANO_TOKEN instead") - // loginWithToken validates token, an account or project access token, for // LoginWithToken and LoginWithDeviceCode. userID is the user a device login // named, if any. func (s Service) loginWithToken(ctx context.Context, cfg *config.Config, token, userID, project string) (Credentials, error) { - // A grant is never saved to disk, so it is refused before any request. - if config.IsDelegatedGrant(token) { - return Credentials{}, ErrDelegatedGrantLogin - } project = strings.TrimSpace(project) apiURL := s.apiURL(cfg) client, err := s.sessions.APIClient(apiURL, token) @@ -221,7 +213,7 @@ func (s Service) loginWithToken(ctx context.Context, cfg *config.Config, token, } origin := projectNamed - if project == "" && config.IsProjectBound(token) { + if project == "" && config.IsProjectToken(token) { project, origin = unnamedProject(cfg) if project == "" { return Credentials{}, projectTokenNeedsProjectError{} @@ -278,7 +270,7 @@ func resolveLoginProject( return selected, nil } - if config.IsProjectBound(token) { + if config.IsProjectToken(token) { return nil, fmt.Errorf("invalid project %q: a project access token (%s) cannot look up a project by name, "+ "so name the project it was minted in by ID", identifier, config.ProjectTokenPrefix) } diff --git a/internal/cmd/auth/auth_test.go b/internal/cmd/auth/auth_test.go index 6f2b81f5..341b354e 100644 --- a/internal/cmd/auth/auth_test.go +++ b/internal/cmd/auth/auth_test.go @@ -19,7 +19,6 @@ import ( "github.com/stretchr/testify/require" "github.com/Kong/volcano-cli/internal/api" - cliauth "github.com/Kong/volcano-cli/internal/auth" cliconfig "github.com/Kong/volcano-cli/internal/config" cliruntime "github.com/Kong/volcano-cli/internal/runtime" ) @@ -223,27 +222,6 @@ func TestLoginRefusesATokenAndADeviceCodeTogether(t *testing.T) { require.ErrorContains(t, err, "none of the others can be") } -// A delegated grant is never saved: login refuses it before any request. -func TestLoginRefusesADelegatedGrant(t *testing.T) { - setAuthTestHome(t) - server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) - })) - defer server.Close() - - for _, token := range []string{cliconfig.DelegatedGrantPrefix + "grant-secret", " PG-grant-secret\n"} { - out, err := executeAuthCommand(t, NewLogin(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}), - "--token", token, "--project", authProjectID) - require.ErrorIs(t, err, cliauth.ErrDelegatedGrantLogin) - require.ErrorContains(t, err, "pass it to each command in VOLCANO_TOKEN") - assert.NotContains(t, err.Error()+out, "grant-secret") - } - - path, err := cliconfig.Path() - require.NoError(t, err) - assert.NoFileExists(t, path, "nothing is saved") -} - // Nothing can discover which project a pt- token belongs to: listing projects // is exactly what it cannot do. Saying so beats saving a credential that then // fails on every command. From 726c499c1fcb4939589da19e7ee383673c6d3d5e Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 11:38:32 -0500 Subject: [PATCH 10/11] fix(auth): name delegated grants in their refusals and document what login does with one --- cmd/volcano/grant_test.go | 2 ++ cmd/volcano/main_test.go | 2 ++ docs/authentication.md | 14 ++++++-- internal/cmd/feedback/feedback.go | 6 +++- internal/cmd/feedback/feedback_test.go | 33 +++++++++++-------- internal/config/config.go | 45 ++++++++++++++++++++++---- internal/config/config_test.go | 14 +++++++- 7 files changed, 92 insertions(+), 24 deletions(-) diff --git a/cmd/volcano/grant_test.go b/cmd/volcano/grant_test.go index f6d13ad1..335a3a5e 100644 --- a/cmd/volcano/grant_test.go +++ b/cmd/volcano/grant_test.go @@ -164,6 +164,8 @@ func TestGrantIsRefusedOnAccountCommandsBeforeAnyRequest(t *testing.T) { assert.Equal(t, exitPermissionDenied, run.code, run.stderr) assert.Contains(t, run.stderr, "this command needs an account token") + assert.Contains(t, run.stderr, "the current credential is a delegated grant (pg-)") + assert.NotContains(t, run.stderr, "project access token") assert.Empty(t, run.requests) }) } diff --git a/cmd/volcano/main_test.go b/cmd/volcano/main_test.go index 640311ae..e5986a86 100644 --- a/cmd/volcano/main_test.go +++ b/cmd/volcano/main_test.go @@ -55,6 +55,8 @@ func TestRunMapsErrorsToStableExitCodes(t *testing.T) { {name: "missing resource", err: fmt.Errorf("get: %w", &api.Error{StatusCode: http.StatusNotFound}), want: exitNotFound}, {name: "refused", err: fmt.Errorf("delete: %w", &api.Error{StatusCode: http.StatusForbidden}), want: exitPermissionDenied}, {name: "account token required", err: cliconfig.ErrAccountTokenRequired, want: exitPermissionDenied}, + {name: "account token required, not a grant", err: cliconfig.ErrAccountTokenRequiredForGrant, want: exitPermissionDenied}, + {name: "no project for a grant", err: cliconfig.ErrNoProjectSelectedForGrant, want: exitNoProject}, {name: "unknown resource", err: fmt.Errorf("frontend %q %w", "web", api.ErrNotFound), want: exitNotFound}, {name: "rewritten 404", err: api.WithCause(errors.New("no such thing"), &api.Error{StatusCode: http.StatusNotFound}), want: exitNotFound}, {name: "rewritten refusal", err: api.WithCause(errors.New("ask a person"), cliconfig.ErrAccountTokenRequired), want: exitPermissionDenied}, diff --git a/docs/authentication.md b/docs/authentication.md index 1d9e0601..f88e8b6d 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -61,8 +61,8 @@ The CLI accepts three kinds of credential: | Delegated grant (`pg-`) | One project, only the commands you consented to | An app you consent to | Account tokens and project access tokens work with `volcano login --token` and -with `VOLCANO_TOKEN`. A delegated grant works only in `VOLCANO_TOKEN`; see -[Delegated grants](#delegated-grants). +with `VOLCANO_TOKEN`. A delegated grant is meant for `VOLCANO_TOKEN`, not for +`volcano login`; see [Delegated grants](#delegated-grants). The prompt on the dashboard's Projects page holds a device code instead of a token. It works with `--device-code` on `volcano login` or `volcano connect`: @@ -111,10 +111,18 @@ project access token. See A delegated grant (`pg-`) is what a Volcano app uses to act on a project you consented to. Like a project access token, it reaches one project; Volcano -also limits it to the commands you consented to. Pass it in `VOLCANO_TOKEN`; don't save it with `volcano login`. +also limits it to the commands you consented to. Pass it in `VOLCANO_TOKEN`. A grant that has used up its changes fails with exit code `6`. See [scripting.md](scripting.md#exit-codes). +`volcano login --token` does not refuse a grant. It checks the grant with +Volcano the way it checks an account token, and Volcano refuses a grant on +most routes. Without `--project`, or with a project name, the check fails and +login saves nothing. With `--project` set to the ID of the grant's own +project, the check passes if the grant may read the project, and login saves +the grant to `~/.volcano/config.json`. A saved grant stops working when the +app's run ends, so keep it in `VOLCANO_TOKEN` instead. + ## Log out Log out at any time: diff --git a/internal/cmd/feedback/feedback.go b/internal/cmd/feedback/feedback.go index 7bab0a4d..a7fd72bb 100644 --- a/internal/cmd/feedback/feedback.go +++ b/internal/cmd/feedback/feedback.go @@ -239,7 +239,11 @@ func printStatus(cmd *cobra.Command) error { fmt.Fprintln(w, "Feedback is enabled, but no report is sent until you log in with `volcano login`.") return nil } - if config.IsProjectBound(cfg.Token()) { + if config.IsDelegatedGrant(cfg.Token()) { + fmt.Fprintln(w, "Feedback is enabled, but a delegated grant cannot send reports. Log in with `volcano login`.") + return nil + } + if config.IsProjectToken(cfg.Token()) { fmt.Fprintln(w, "Feedback is enabled, but a project access token cannot send reports. Log in with `volcano login`.") return nil } diff --git a/internal/cmd/feedback/feedback_test.go b/internal/cmd/feedback/feedback_test.go index a2610ae6..81af0bdc 100644 --- a/internal/cmd/feedback/feedback_test.go +++ b/internal/cmd/feedback/feedback_test.go @@ -309,22 +309,29 @@ func TestFeedback_LoggedOutSendsNothing(t *testing.T) { } func TestFeedback_ProjectTokenSendsNothing(t *testing.T) { - reports := setup(t, &config.TelemetryConfig{DeviceID: "d", NoticeShown: true}) - t.Setenv("VOLCANO_TOKEN", "pt-abc") + for token, credential := range map[string]string{ + "pt-abc": "a project access token", + "pg-abc": "a delegated grant", + } { + t.Run(token, func(t *testing.T) { + reports := setup(t, &config.TelemetryConfig{DeviceID: "d", NoticeShown: true}) + t.Setenv("VOLCANO_TOKEN", token) - stdout, stderr, err := run(t, reportArgs...) - require.NoError(t, err, "exit 0, so an agent does not retry") - assert.Empty(t, stdout) - assert.Empty(t, stderr) - assert.Empty(t, *reports) + stdout, stderr, err := run(t, reportArgs...) + require.NoError(t, err, "exit 0, so an agent does not retry") + assert.Empty(t, stdout) + assert.Empty(t, stderr) + assert.Empty(t, *reports) - stdout, _, err = run(t, "status") - require.NoError(t, err) - assert.Equal(t, "Feedback is enabled, but a project access token cannot send reports. Log in with `volcano login`.\n", stdout) + stdout, _, err = run(t, "status") + require.NoError(t, err) + assert.Equal(t, "Feedback is enabled, but "+credential+" cannot send reports. Log in with `volcano login`.\n", stdout) - _, stderr, err = run(t, append([]string{"--dry-run"}, reportArgs...)...) - require.NoError(t, err) - assert.Contains(t, stderr, "This report would not be sent.") + _, stderr, err = run(t, append([]string{"--dry-run"}, reportArgs...)...) + require.NoError(t, err) + assert.Contains(t, stderr, "This report would not be sent.") + }) + } } func TestFeedback_SettingSurvivesLogout(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index e1234493..cb2b6a76 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -55,8 +55,35 @@ var ( ErrAccountTokenRequired = errors.New("this command needs an account token (" + AccountTokenPrefix + ") but the current credential is a project access token (" + ProjectTokenPrefix + "), which only reaches the project it was minted in. Run 'volcano login', or set VOLCANO_TOKEN to an account token") + // ErrNoProjectSelectedForGrant is ErrNoProjectSelectedForProjectToken + // worded for a delegated grant. It matches that error, so the exit code is + // the same. + ErrNoProjectSelectedForGrant error = grantRefusal{ + message: "no project selected. A delegated grant (" + DelegatedGrantPrefix + + ") is scoped to one project: run 'volcano use ' with that project's ID", + matches: ErrNoProjectSelectedForProjectToken, + } + // ErrAccountTokenRequiredForGrant is ErrAccountTokenRequired worded for a + // delegated grant. It matches that error, so the exit code is the same. + ErrAccountTokenRequiredForGrant error = grantRefusal{ + message: "this command needs an account token (" + AccountTokenPrefix + + ") but the current credential is a delegated grant (" + DelegatedGrantPrefix + + "), which only reaches the project it was granted for. Run 'volcano login', or set VOLCANO_TOKEN to an account token", + matches: ErrAccountTokenRequired, + } ) +// grantRefusal is a refusal worded for a delegated grant that matches the +// project access token's refusal with errors.Is. +type grantRefusal struct { + message string + matches error +} + +func (e grantRefusal) Error() string { return e.message } + +func (e grantRefusal) Is(target error) bool { return target == e.matches } + // These variables are intentionally settable with -ldflags -X. var ( compiledDefaultAPIURL = defaultCompiledAPIURL @@ -625,18 +652,24 @@ func (c *Config) RequireProject() error { if strings.TrimSpace(c.ProjectID()) != "" { return nil } - if IsProjectBound(c.Token()) { + if IsDelegatedGrant(c.Token()) { + return ErrNoProjectSelectedForGrant + } + if IsProjectToken(c.Token()) { return ErrNoProjectSelectedForProjectToken } return ErrNoProjectSelected } -// RequireAccountToken rejects a project access token on a command that needs -// account-wide access. Any other credential is treated as an account token: the -// server is the authority on what a token may do, and tokens minted before the -// prefixes existed carry neither. +// RequireAccountToken rejects a project access token or a delegated grant on a +// command that needs account-wide access. Any other credential is treated as an +// account token: the server is the authority on what a token may do, and tokens +// minted before the prefixes existed carry none. func (c *Config) RequireAccountToken() error { - if IsProjectBound(c.Token()) { + if IsDelegatedGrant(c.Token()) { + return ErrAccountTokenRequiredForGrant + } + if IsProjectToken(c.Token()) { return ErrAccountTokenRequired } return nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ee9b7c21..b5fb5b91 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -532,11 +532,23 @@ func TestIsDelegatedGrantAndIsProjectBound(t *testing.T) { } // A grant was once an unknown prefix, and so an account token. It is refused -// where a pt- token is, with the same sentinels. +// where a pt- token is, with the same sentinels, in words that name a grant. func TestRequireAccountTokenAndProjectRefuseADelegatedGrant(t *testing.T) { t.Setenv(envToken, "") t.Setenv(envProjectID, "") cfg := &Config{UserToken: DelegatedGrantPrefix + "secret-grant"} + for name, err := range map[string]error{ + "account token": cfg.RequireAccountToken(), + "project": cfg.RequireProject(), + } { + t.Run(name, func(t *testing.T) { + require.Error(t, err) + assert.Contains(t, err.Error(), "delegated grant ("+DelegatedGrantPrefix+")") + assert.NotContains(t, err.Error(), "project access token") + assert.NotContains(t, err.Error(), ProjectTokenPrefix) + }) + } require.ErrorIs(t, cfg.RequireAccountToken(), ErrAccountTokenRequired) require.ErrorIs(t, cfg.RequireProject(), ErrNoProjectSelectedForProjectToken) + assert.NotErrorIs(t, cfg.RequireProject(), ErrAccountTokenRequired) } From 55e9a0e3fb963f0fe7770bcb67557d2e6c6b11bb Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 11:51:30 -0500 Subject: [PATCH 11/11] fix(telemetry): redact vgt_ run tokens and name delegated grants in feedback, LavaFlow and approval refusals --- docs/domains.md | 3 ++- docs/durable-functions.md | 4 +-- docs/feedback.md | 7 +++--- docs/git.md | 3 ++- docs/lavaflow.md | 11 ++++---- docs/telemetry.md | 2 +- internal/cmd/connect/connect.go | 4 +-- internal/cmd/durable/approvals/approvals.go | 2 +- .../cmd/durable/approvals/approvals_test.go | 24 ++++++++++++------ internal/cmd/durable/approvals/approve.go | 2 +- internal/cmd/durable/approvals/deny.go | 2 +- internal/cmd/feedback/feedback.go | 15 +++++------ internal/cmd/lavaflow/disable.go | 2 +- internal/cmd/lavaflow/enable.go | 3 ++- internal/cmd/lavaflow/enable_test.go | 6 ++--- internal/cmd/lavaflow/runs_start_test.go | 25 +++++++++++-------- internal/cmd/lavaflow/settings_set.go | 2 +- internal/durable/durable.go | 9 ++++--- internal/lavaflow/lavaflow.go | 7 +++--- internal/lavaflow/start.go | 8 +++--- internal/telemetry/fields.go | 2 +- internal/telemetry/fields_test.go | 13 +++++----- 22 files changed, 90 insertions(+), 66 deletions(-) diff --git a/docs/domains.md b/docs/domains.md index 5c2e476c..5caea749 100644 --- a/docs/domains.md +++ b/docs/domains.md @@ -13,7 +13,8 @@ projects, with no further record. ## How it relates - Belongs to your **account**, not a project, so the commands need an account - token from `volcano login`. A project access token is refused. + token from `volcano login`. A project access token or a delegated grant is + refused. - Every **frontend custom domain** must sit at or below a domain your account has verified. `volcano cloud frontends domains add`, `volcano cloud frontends domain create`, and `volcano cloud config deploy` print the record to publish diff --git a/docs/durable-functions.md b/docs/durable-functions.md index 8b59670d..a45e750f 100644 --- a/docs/durable-functions.md +++ b/docs/durable-functions.md @@ -242,8 +242,8 @@ any decision on an expired or cancelled approval, fails with what happened, for example `approval … was already approved by owner@example.com at …`. An approval past its timeout counts as expired, even while it still reads `pending`. -Approvals are decided by a person. A command run with a project access token is -refused before it asks; run `volcano login` with `VOLCANO_TOKEN` unset, then +Approvals are decided by a person. A command run with a project access token or +a delegated grant is refused before it asks; run `volcano login` with `VOLCANO_TOKEN` unset, then decide again. `stats` counts the approvals requested in a window, 30 days unless `--since` diff --git a/docs/feedback.md b/docs/feedback.md index 6c5692df..06defdf6 100644 --- a/docs/feedback.md +++ b/docs/feedback.md @@ -35,8 +35,8 @@ volcano feedback \ --workaround "Parsed the table output instead." ``` -Sending a report requires `volcano login`. A project access token cannot send -one. See [When a report is not sent](#when-a-report-is-not-sent). +Sending a report requires `volcano login`. A project access token or a +delegated grant cannot send one. See [When a report is not sent](#when-a-report-is-not-sent). | Flag | Required | Limit | Content | |---|---|---|---| @@ -137,7 +137,8 @@ environment variable values, application data, or customer names in a report. when this run shows it, and exits 0 when: - Feedback is off. See [Turn it off](#turn-it-off). -- You are not logged in, or are logged in only with a project access token. +- You are not logged in, or your credential is a project access token or a + delegated grant. - The first-run notice has not been shown yet. The first `volcano` command on a machine shows it, and reports are sent from the next command on. Where `~/.volcano/config.json` cannot be saved, such as a read-only home, the diff --git a/docs/git.md b/docs/git.md index 946108da..b491af70 100644 --- a/docs/git.md +++ b/docs/git.md @@ -23,7 +23,8 @@ into an empty repository; Volcano pushes that first commit itself. set up either yet; see [Prerequisites the CLI cannot set up](#prerequisites-the-cli-cannot-set-up). - `connect` reads that account-wide GitHub connection, so it needs an account - token (`pk-`) and says so when run with a project access token. Which + token (`pk-`) and says so when run with a project access token or a + delegated grant. Which repository a project deploys from is a person's choice. - `root-directory` changes only the subdirectory the connected repository builds from, and works with a project access token, so a CI job or an agent diff --git a/docs/lavaflow.md b/docs/lavaflow.md index 0a2a9c3f..02837a5d 100644 --- a/docs/lavaflow.md +++ b/docs/lavaflow.md @@ -74,10 +74,9 @@ LavaFlow belongs to the project's owner. For anyone else, every command says code 4. A project access token can run the read commands for its own project, but only the owner's own session can turn LavaFlow on or off, change its settings and start runs: `enable`, `disable`, `settings set` and -`runs start` refuse a project access token before they ask or send anything, -with code 5. Sign in with -`volcano login` instead, and if `VOLCANO_TOKEN` holds a project access token, -unset it first. +`runs start` refuse a project access token or a delegated grant before they +ask or send anything, with code 5. Sign in with `volcano login` instead, and if +`VOLCANO_TOKEN` holds either, unset it first. When the owner's plan can't be read, a command says "LavaFlow couldn't read the project owner's plan; try again later" and exits with code 5. A change @@ -220,8 +219,8 @@ ended, an owner whose GitHub account can't push to the repository, or a project with no repository. The command says why in Volcano's words and exits with code 5. [Checking that the owner can push](/platform/projects/git-deploy#check-that-the-owner-can-push) says why the owner can't. While `VOLCANO_TOKEN` is set, its token is used -instead of a `volcano login` sign-in. A project access token is refused before -anything is sent, also with code 5. Merging applies to the +instead of a `volcano login` sign-in. A project access token or a delegated +grant is refused before anything is sent, also with code 5. Merging applies to the repository the project is connected to when you turn it on: once the project is connected to another, autonomy reads `open_pr` until you turn merge on again. Merging is only for a project where diff --git a/docs/telemetry.md b/docs/telemetry.md index b3388fb4..a5fbbb3d 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -40,7 +40,7 @@ When a command fails, the event also carries the type of failure (`api`, code of an API error, and the error message. Before sending the message, the CLI replaces your home directory with `~`, strips terminal color codes, and removes bearer tokens, JWTs, Volcano keys and tokens (`pk-`, `pt-`, `pg-`, `sk-`, -`ak-`, `vbt_`, `vpg_`, `vcb_`, `sbp1.`), passwords in URLs, email addresses, and URL +`ak-`, `vbt_`, `vpg_`, `vcb_`, `vgt_`, `sbp1.`), passwords in URLs, email addresses, and URL query strings. It then cuts the message to 500 characters. Some errors can quote your files or data, so the CLI replaces that part with diff --git a/internal/cmd/connect/connect.go b/internal/cmd/connect/connect.go index 7cd8c41d..3da8e3bb 100644 --- a/internal/cmd/connect/connect.go +++ b/internal/cmd/connect/connect.go @@ -384,8 +384,8 @@ func ensureSession(ctx context.Context, opts options, sessions clisession.Factor } if token := cfg.Token(); token != "" && !opts.signup && !refused { - // A project access token can't be checked without a project; the - // project lookup below reports it if it is wrong. + // A project access token or a delegated grant can't be checked without a + // project; the project lookup below reports it if it is wrong. if config.IsProjectBound(token) { return nil } diff --git a/internal/cmd/durable/approvals/approvals.go b/internal/cmd/durable/approvals/approvals.go index 2dec2d3c..8962616c 100644 --- a/internal/cmd/durable/approvals/approvals.go +++ b/internal/cmd/durable/approvals/approvals.go @@ -31,7 +31,7 @@ one with ctx.waitForApproval and suspends until a person approves or denies it, or until its timeout passes. Either way the workflow resumes with the outcome. Anyone with access to the project can list approvals and read their stats. -Only a person can decide one: approve and deny refuse a project access token.`, +Only a person can decide one: approve and deny refuse a project access token or a delegated grant.`, } cmd.AddCommand(newList(deps)) cmd.AddCommand(newGet(deps)) diff --git a/internal/cmd/durable/approvals/approvals_test.go b/internal/cmd/durable/approvals/approvals_test.go index 07c8a708..431d1759 100644 --- a/internal/cmd/durable/approvals/approvals_test.go +++ b/internal/cmd/durable/approvals/approvals_test.go @@ -867,15 +867,25 @@ func TestApprovalsDecideRefusesAProjectAccessToken(t *testing.T) { } } -// A credential the CLI knows is a project access token is refused before the -// prompt and before the approval is read: confirming a decision the API will -// refuse wastes the person's yes, and an approval that does not exist would -// otherwise answer with a 404 instead of saying who can decide. +// A credential the CLI knows is a project access token or a delegated grant is +// refused before the prompt and before the approval is read: confirming a +// decision the API will refuse wastes the person's yes, and an approval that +// does not exist would otherwise answer with a 404 instead of saying who can +// decide. func TestApprovalsDecideRefusesAProjectTokenBeforeAnyRequest(t *testing.T) { - for _, args := range [][]string{{"approve", approvalID}, {"deny", approvalID, "--yes"}} { - t.Run(args[0], func(t *testing.T) { + for _, tc := range []struct{ token, command string }{ + {"PT-approvals-test", "approve"}, + {"PT-approvals-test", "deny"}, + {"pg-approvals-test", "approve"}, + {"pg-approvals-test", "deny"}, + } { + args := []string{"approve", approvalID} + if tc.command == "deny" { + args = []string{"deny", approvalID, "--yes"} + } + t.Run(tc.token[:3]+tc.command, func(t *testing.T) { setApprovalsTestHome(t) - t.Setenv("VOLCANO_TOKEN", "PT-approvals-test") + t.Setenv("VOLCANO_TOKEN", tc.token) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Errorf("a project access token's decision must not reach the API: %s %s", r.Method, r.URL.Path) respondJSON(t, w, http.StatusNotFound, map[string]any{"error": "not found"}) diff --git a/internal/cmd/durable/approvals/approve.go b/internal/cmd/durable/approvals/approve.go index 8646e065..d4e22c1b 100644 --- a/internal/cmd/durable/approvals/approve.go +++ b/internal/cmd/durable/approvals/approve.go @@ -22,7 +22,7 @@ was denied, expired, or cancelled cannot be approved; the command says which and exits non-zero. Only a person can decide: log in with 'volcano login'. A project access token -is refused. +or a delegated grant is refused. By default this command prompts for confirmation, showing the approval's title. Use --yes to skip the prompt.`, diff --git a/internal/cmd/durable/approvals/deny.go b/internal/cmd/durable/approvals/deny.go index ff83d340..ab3040a6 100644 --- a/internal/cmd/durable/approvals/deny.go +++ b/internal/cmd/durable/approvals/deny.go @@ -21,7 +21,7 @@ approved, expired, or cancelled cannot be denied; the command says which and exits non-zero. Only a person can decide: log in with 'volcano login'. A project access token -is refused. +or a delegated grant is refused. By default this command prompts for confirmation, showing the approval's title. Use --yes to skip the prompt.`, diff --git a/internal/cmd/feedback/feedback.go b/internal/cmd/feedback/feedback.go index a7fd72bb..eac46f18 100644 --- a/internal/cmd/feedback/feedback.go +++ b/internal/cmd/feedback/feedback.go @@ -77,13 +77,14 @@ carries the selected project ID and this machine's telemetry ID, and is linked to your account. With telemetry off and feedback turned on with volcano feedback enable, it carries neither and is not linked to your account. Every report is sent with your login, which the server checks, so sending one -requires volcano login. A project access token cannot send reports. +requires volcano login. A project access token or a delegated grant cannot +send reports. A report is not sent, and the command exits 0 with no output other than the first-run notice when this run shows it, while feedback is off, when you are -not logged in or use a project access token, or before the first-run notice -has been shown. --dry-run says whether a report would be sent, and volcano -feedback status says why not. +not logged in or use a project access token or a delegated grant, or before +the first-run notice has been shown. --dry-run says whether a report would be +sent, and volcano feedback status says why not. Feedback follows telemetry: turning telemetry off also turns feedback off, unless you run volcano feedback enable. Turn it off with volcano feedback @@ -127,8 +128,8 @@ disable or VOLCANO_FEEDBACK_DISABLED=1.`, return nil } // The server takes reports only from a logged-in account, so a - // logged-out run, or one with a project access token, sends - // nothing, like feedback that is off. + // logged-out run, or one with a project access token or a + // delegated grant, sends nothing, like feedback that is off. token := cfg.Token() loggedIn := telemetry.CanSendFeedback(token) if !loggedIn && !dryRun { @@ -342,7 +343,7 @@ func saveReport(payload []byte) error { // printReport prints report for --dry-run, and says so when a real run would // send nothing: feedback is off, the first-run notice has not been shown, or -// the user is logged out or uses a project access token. +// the user is logged out or uses a project access token or a delegated grant. func printReport(cmd *cobra.Command, report feedback.Report, sends bool) error { out, err := encodeReport(report, " ") if err != nil { diff --git a/internal/cmd/lavaflow/disable.go b/internal/cmd/lavaflow/disable.go index 49a0ed19..d8cbfd81 100644 --- a/internal/cmd/lavaflow/disable.go +++ b/internal/cmd/lavaflow/disable.go @@ -24,7 +24,7 @@ func newDisable(deps cliruntime.Deps) *cobra.Command { Short: "Turn LavaFlow off", Long: `Turn LavaFlow off for the current project. No new run starts; past runs and their records stay readable. Only the project's owner can, from their own -session: a project access token is refused.`, +session: a project access token or a delegated grant is refused.`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { opts.deps = deps diff --git a/internal/cmd/lavaflow/enable.go b/internal/cmd/lavaflow/enable.go index e6d03b3b..bf5feb86 100644 --- a/internal/cmd/lavaflow/enable.go +++ b/internal/cmd/lavaflow/enable.go @@ -38,7 +38,8 @@ This turns LavaFlow on for the current project. Before it does, it prints the sentence above and: ` + clilavaflow.EnableNotice + ` and asks you to confirm; --yes skips the question. Only the project's owner -can turn it on, from their own session: a project access token is refused. The +can turn it on, from their own session: a project access token or a delegated +grant is refused. The owner's plan has to include LavaFlow. Connect a repository with 'volcano git connect' so LavaFlow can open fix pull requests. Choose how far it goes with '` + cliruntime.CommandPath(deps, "lavaflow settings set --autonomy") + `'.`, diff --git a/internal/cmd/lavaflow/enable_test.go b/internal/cmd/lavaflow/enable_test.go index 71e7e073..d5c4ab0d 100644 --- a/internal/cmd/lavaflow/enable_test.go +++ b/internal/cmd/lavaflow/enable_test.go @@ -58,7 +58,7 @@ func TestChangesWithAProjectTokenNeedAPersonsSession(t *testing.T) { t.Helper() t.Setenv("VOLCANO_TOKEN", cliconfig.ProjectTokenPrefix+"secret") }, - "changing LavaFlow's settings needs a person's session: a project access token can't. " + + "changing LavaFlow's settings needs a person's session: a project access token or a delegated grant can't. " + "Unset VOLCANO_TOKEN and sign in with 'volcano login', then run this again", }, "saved": { @@ -69,7 +69,7 @@ func TestChangesWithAProjectTokenNeedAPersonsSession(t *testing.T) { CurrentProject: &cliconfig.ProjectConfig{ID: lavaflowProjectID, Name: "Shop"}, }).Save()) }, - "changing LavaFlow's settings needs a person's session: a project access token can't. " + + "changing LavaFlow's settings needs a person's session: a project access token or a delegated grant can't. " + "Sign in with 'volcano login' and run this again", }, } { @@ -216,7 +216,7 @@ func TestAPersonRequiredRefusalKeepsVolcanosWords(t *testing.T) { for message, want := range map[string]string{ tokenNeverExpiresRefusal: "LavaFlow's settings weren't changed: " + tokenNeverExpiresRefusal, signInRefusal: "LavaFlow's settings weren't changed: " + signInRefusal, - "": "changing LavaFlow's settings needs a person's session: a project access token can't. " + + "": "changing LavaFlow's settings needs a person's session: a project access token or a delegated grant can't. " + "Sign in with 'volcano login' and run this again", } { fake, deps := newLavaflowServer(t) diff --git a/internal/cmd/lavaflow/runs_start_test.go b/internal/cmd/lavaflow/runs_start_test.go index 2dbba362..410140ec 100644 --- a/internal/cmd/lavaflow/runs_start_test.go +++ b/internal/cmd/lavaflow/runs_start_test.go @@ -50,18 +50,23 @@ func TestRunStartRefusesAnInvalidIssueID(t *testing.T) { fake.set(func(s *lavaflowServer) { assert.Empty(t, s.started) }) } -// A project access token can't start a run, so the CLI says how the owner -// does before it sends anything, with the exit code of a permission refused. +// A project access token or a delegated grant can't start a run, so the CLI +// says how the owner does before it sends anything, with the exit code of a +// permission refused. func TestRunStartWithAProjectTokenNeedsTheOwnersSession(t *testing.T) { - fake, deps := newLavaflowServer(t) - t.Setenv("VOLCANO_TOKEN", cliconfig.ProjectTokenPrefix+"secret") + for _, prefix := range []string{cliconfig.ProjectTokenPrefix, cliconfig.DelegatedGrantPrefix} { + t.Run(prefix, func(t *testing.T) { + fake, deps := newLavaflowServer(t) + t.Setenv("VOLCANO_TOKEN", prefix+"secret") - _, err := executeLavaflowCommand(t, New(deps), "runs", "start", lavaflowIssueID) - require.ErrorIs(t, err, clilavaflow.ErrPersonStarts) - require.ErrorIs(t, err, cliconfig.ErrAccountTokenRequired, "the exit code reports a permission refused") - assert.Equal(t, "starting a LavaFlow run needs the project owner's session: a project access token can't. "+ - "Unset VOLCANO_TOKEN and sign in with 'volcano login', then run this again", err.Error()) - fake.set(func(s *lavaflowServer) { assert.Empty(t, s.started) }) + _, err := executeLavaflowCommand(t, New(deps), "runs", "start", lavaflowIssueID) + require.ErrorIs(t, err, clilavaflow.ErrPersonStarts) + require.ErrorIs(t, err, cliconfig.ErrAccountTokenRequired, "the exit code reports a permission refused") + assert.Equal(t, "starting a LavaFlow run needs the project owner's session: a project access token or a delegated grant can't. "+ + "Unset VOLCANO_TOKEN and sign in with 'volcano login', then run this again", err.Error()) + fake.set(func(s *lavaflowServer) { assert.Empty(t, s.started) }) + }) + } } // Each refusal says what to do about it, keeping the API's status for the diff --git a/internal/cmd/lavaflow/settings_set.go b/internal/cmd/lavaflow/settings_set.go index 20c2e87d..884bde6e 100644 --- a/internal/cmd/lavaflow/settings_set.go +++ b/internal/cmd/lavaflow/settings_set.go @@ -39,7 +39,7 @@ func newSettingsSet(deps cliruntime.Deps) *cobra.Command { Short: "Change LavaFlow's settings", Long: fmt.Sprintf(`Change LavaFlow's settings for the current project. A setting you don't pass keeps its value. Only the project's owner can, from their own session: a -project access token is refused. +project access token or a delegated grant is refused. --autonomy decides how far LavaFlow goes: %[1]s diagnoses issues and leaves any fix to a person; %[2]s also opens fix pull requests for review; %[4]s diff --git a/internal/durable/durable.go b/internal/durable/durable.go index 977cbc31..3a2ee815 100644 --- a/internal/durable/durable.go +++ b/internal/durable/durable.go @@ -487,10 +487,11 @@ func (s Service) DecideApproval( return nil, approvalError(action, approvalID, err) } -// errApprovalNeedsAPerson is the answer to a project access token deciding. +// errApprovalNeedsAPerson is the answer to a project access token or a +// delegated grant deciding. // Approvals exist so that a person signs off, which is why the API refuses // any credential that is not a person's. VOLCANO_TOKEN overrides a login, and -// is how a project access token usually arrives. It matches +// is how either usually arrives. It matches // config.ErrAccountTokenRequired, so the exit code reports a refused // credential. The API's 403 is left out: its generic body would add a hint // about the token's project, which is not the problem here. @@ -498,8 +499,8 @@ var errApprovalNeedsAPerson = api.WithCause(errors.New( "approvals are decided by a person. Run `volcano login` with VOLCANO_TOKEN unset, then decide again"), config.ErrAccountTokenRequired) -// RequirePerson refuses a credential the CLI knows is a project access token, -// so a decision is not confirmed only to be refused. DecideApproval still maps +// RequirePerson refuses a credential the CLI knows is a project access token +// or a delegated grant, so a decision is not confirmed only to be refused. DecideApproval still maps // the API's refusal, for credentials without a recognizable prefix. func (s Service) RequirePerson() error { cfg, err := s.sessions.Config() diff --git a/internal/lavaflow/lavaflow.go b/internal/lavaflow/lavaflow.go index 974b1e2e..e54f6433 100644 --- a/internal/lavaflow/lavaflow.go +++ b/internal/lavaflow/lavaflow.go @@ -106,10 +106,10 @@ var ErrCantTurnOn = errors.New("LavaFlow can't be turned on for this project: " "it isn't open to the owner's account, or the owner's plan doesn't include it") // ErrPersonDecides refuses changing LavaFlow's settings with a project access -// token. Turning LavaFlow on, off or changing how far it goes is a person's +// token or a delegated grant. Turning LavaFlow on, off or changing how far it goes is a person's // decision, so the API takes it from a person's session only, and the CLI // says so before asking. -var ErrPersonDecides = errors.New("changing LavaFlow's settings needs a person's session: a project access token can't") +var ErrPersonDecides = errors.New("changing LavaFlow's settings needs a person's session: a project access token or a delegated grant can't") // ErrOwnerDecides refuses changing LavaFlow's settings when the settings say // the caller can't, before the CLI asks or sends anything. Only the project's @@ -415,7 +415,8 @@ func validMergePolicy(policy *apiclient.LavaflowMergePolicyUpdate) error { return nil } -// update refuses a project access token before sending anything. Every +// update refuses a project access token or a delegated grant before sending +// anything. Every // change reads the settings first, which refuses a project without LavaFlow // records with the route's reason, and checks can_update before it asks or // sends. A change that turns LavaFlow on, or changes autonomy to merge, then diff --git a/internal/lavaflow/start.go b/internal/lavaflow/start.go index a9bbe274..4060ff8e 100644 --- a/internal/lavaflow/start.go +++ b/internal/lavaflow/start.go @@ -16,14 +16,16 @@ import ( clisession "github.com/Kong/volcano-cli/internal/session" ) -// ErrPersonStarts refuses starting a run with a project access token. A run +// ErrPersonStarts refuses starting a run with a project access token or a +// delegated grant. A run // sends the project's logs and source code to an AI service and can open a // pull request, so the API takes it from the owner's own session only. -var ErrPersonStarts = errors.New("starting a LavaFlow run needs the project owner's session: a project access token can't") +var ErrPersonStarts = errors.New("starting a LavaFlow run needs the project owner's session: a project access token or a delegated grant can't") // ErrStartNeedsLiveSignIn reports the start's person_required for a token // the CLI sent as a person's: a platform token that never expires, or one -// whose sign-in ended. The CLI refuses a project access token before it asks. +// whose sign-in ended. The CLI refuses a project access token or a delegated +// grant before it asks. var ErrStartNeedsLiveSignIn = errors.New("starting a LavaFlow run needs the project owner's live sign-in") // ErrStartUnavailable reports an API that serves LavaFlow but can't start a diff --git a/internal/telemetry/fields.go b/internal/telemetry/fields.go index c6d8fb0d..81ad7768 100644 --- a/internal/telemetry/fields.go +++ b/internal/telemetry/fields.go @@ -177,7 +177,7 @@ const redacted = "[redacted]" var ( bearerPattern = regexp.MustCompile(`(?i)\bbearer\s+\S+`) jwtPattern = regexp.MustCompile(`eyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]*`) - keyPattern = regexp.MustCompile(`\b(?:(?:(?:pk|pt|pg|sk|ak)-|vbt_|vpg_|vcb_)[A-Za-z0-9_-]{16,}|sbp1\.[A-Za-z0-9+/=:]{16,})`) + keyPattern = regexp.MustCompile(`\b(?:(?:(?:pk|pt|pg|sk|ak)-|vbt_|vpg_|vcb_|vgt_)[A-Za-z0-9_-]{16,}|sbp1\.[A-Za-z0-9+/=:]{16,})`) userPattern = regexp.MustCompile(`([a-zA-Z][a-zA-Z0-9+.-]*://)[^\s/?#]*@`) emailPattern = regexp.MustCompile(`[A-Za-z0-9._%+-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}`) queryPattern = regexp.MustCompile(`(https?://[^\s?#]+)\?[^\s#]*`) diff --git a/internal/telemetry/fields_test.go b/internal/telemetry/fields_test.go index b87ca65a..4d5b6560 100644 --- a/internal/telemetry/fields_test.go +++ b/internal/telemetry/fields_test.go @@ -99,12 +99,13 @@ func TestResources_Capped(t *testing.T) { func TestRedact(t *testing.T) { jwt := "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxIn0.c2lnbmF0dXJl" cases := map[string]string{ - "Authorization: Bearer abc.def-ghi": "Authorization: Bearer [redacted]", - "token " + jwt + " expired": "token [redacted] expired", - "key pk-0123456789abcdefABCD rejected": "key [redacted] rejected", - "used pt-0123456789abcdefABCD, sk-0123456789abcdefABCD": "used [redacted], [redacted]", - "grant pg-0123456789abcdefABCD is spent": "grant [redacted] is spent", - "pk-short": "pk-short", + "Authorization: Bearer abc.def-ghi": "Authorization: Bearer [redacted]", + "token " + jwt + " expired": "token [redacted] expired", + "key pk-0123456789abcdefABCD rejected": "key [redacted] rejected", + "used pt-0123456789abcdefABCD, sk-0123456789abcdefABCD": "used [redacted], [redacted]", + "grant pg-0123456789abcdefABCD is spent": "grant [redacted] is spent", + "x-api-key vgt_0123456789abcdefABCD refused": "x-api-key [redacted] refused", + "pk-short": "pk-short", "proxy sbp1.v2:0123456789abcdef:AbC+dEf/GhI=jKl= denied": "proxy [redacted] denied", "proxy sbp1.AbC+dEf/GhI0123456789= denied": "proxy [redacted] denied", "exchange vbt_0123456789abcdefABCD failed": "exchange [redacted] failed",