From 432d1390716f149a6cb2f39848cab3c59b8bdb45 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 18 Oct 2026 17:40:06 +0200 Subject: [PATCH] fix: address verified review findings (actions-workflows-values-1) Signed-off-by: bircni --- docs/migration-1.0.md | 9 ++-- internal/app/action_values.go | 42 +++++++++++----- internal/app/action_values_test.go | 41 ++++++++++++++-- internal/app/alias_completion_test.go | 30 ++++++++++++ internal/app/completion.go | 16 ++++-- internal/app/validation_test.go | 1 + internal/services/action_values.go | 30 +++++++++--- internal/services/action_values_test.go | 65 ++++++++++++++++++++++++- internal/services/actions.go | 6 +++ internal/services/actions_test.go | 26 ++++++++++ main.go | 3 +- main_test.go | 33 +++++++++++++ 12 files changed, 268 insertions(+), 34 deletions(-) diff --git a/docs/migration-1.0.md b/docs/migration-1.0.md index 14e27f3e..d1f89548 100644 --- a/docs/migration-1.0.md +++ b/docs/migration-1.0.md @@ -284,7 +284,7 @@ changed meaning: | `repo delete` | `--name NAME --owner OWNER` | optional `OWNER/REPO` argument (or `--repo`) | | `pulls close\|reopen\|edit` | several indexes | `pr close\|reopen\|edit` take one pull request (number, URL or branch); loop to change several | | `issue comment` | optional body argument | `--body`, `--body-file` or `--editor` | -| `secret set`, `variable set` | ` []`, `--file`, `--stdin` | value from `--body`, `--body-file FILE\|-` or piped standard input | +| `secret set`, `variable set` | optional value argument, `--file`, `--stdin` | `--body VALUE`, `--body-file FILE` (or `-`), or standard input; one trailing line break is removed | A number is read as an ID; pass `--by-name` to match a milestone, label or asset whose name is a number. Destructive commands still ask for confirmation unless @@ -409,9 +409,10 @@ archive, or run `make docs`. - More input is checked before anything is sent: release titles, issue titles and comment bodies cannot be empty, a review needs a body or inline comments (unless it approves), team creation needs a permission, migrations from - GitHub, Gitea, GitLab and Gogs need an access token, and `workflow run` must find - its workflow and check the inputs (without `--ref` it uses the checked-out - branch, else the default branch). + GitHub, Gitea, GitLab and Gogs need an access token. `workflow run` uses the + checked-out branch (which must be on the server) or the default branch when + there is no `--ref`, and refuses unknown inputs, missing required inputs and + values that a choice, boolean or number input does not accept. - `pr merge` merges the head it read and refuses when the branch moved in the meantime; run it again to merge the new head. - Preferences follow the git/gh order: an explicit `tea config set` value wins diff --git a/internal/app/action_values.go b/internal/app/action_values.go index 55ca5483..405e30ec 100644 --- a/internal/app/action_values.go +++ b/internal/app/action_values.go @@ -11,6 +11,7 @@ import ( "io" "regexp" "strings" + "unicode/utf8" "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/services" @@ -176,6 +177,10 @@ $ echo -n "$TOKEN" | tea secret set NPM_TOKEN --org acme # set the secrets of a dotenv file $ tea secret set --env-file .env` } + descriptionNote := "" + if secret { + descriptionNote = "\n\nSetting an existing secret without --description clears its description." + } cmd := &cobra.Command{ Use: "set []", Short: "Create or update a " + kind, @@ -183,7 +188,7 @@ $ tea secret set --env-file .env` The value comes from --body, --body-file or standard input when it is not a terminal (one trailing line break is removed), or else from a prompt. --env-file sets every NAME=value of a -dotenv file instead.`, kind), +dotenv file instead.%[2]s`, kind, descriptionNote), Example: example, Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { @@ -193,6 +198,11 @@ dotenv file instead.`, kind), if envFile != "" && (cmd.Flags().Changed("body") || cmd.Flags().Changed("body-file")) { return clierr.Wrap(clierr.Usage, errors.New("--env-file cannot be combined with --body or --body-file")) } + if envFile == "" { + if err := checkActionValueName(args[0]); err != nil { + return err + } + } var entries []envEntry if envFile != "" { data, err := readFileOrStdin(a.deps.Streams.In, envFile) @@ -216,8 +226,8 @@ dotenv file instead.`, kind), if entry.value == "" { return clierr.Wrap(clierr.Usage, fmt.Errorf("the value of %s is empty", entry.name)) } - if !secret && len(entry.value) > 65536 { - return clierr.Wrap(clierr.Usage, fmt.Errorf("the value of %s exceeds 64 KiB", entry.name)) + if !secret && utf8.RuneCountInString(entry.value) > 65536 { + return clierr.Wrap(clierr.Usage, fmt.Errorf("the value of %s exceeds 65536 characters", entry.name)) } } selected, t, err := scope.resolve(a, cmd) @@ -244,8 +254,12 @@ dotenv file instead.`, kind), }, func(item services.ActionSecret) string { return report(item.Name) }) } service := services.ActionVariables{Client: t.client, Scope: selected} + var newDescription *string + if cmd.Flags().Changed("description") { + newDescription = &description + } return batchDone(a, cmd, entries, label, func(entry envEntry) (services.ActionVariable, error) { - return service.Set(cmd.Context(), entry.name, entry.value, description) + return service.Set(cmd.Context(), entry.name, entry.value, newDescription) }, func(item services.ActionVariable) string { return report(item.Name) }) }, } @@ -304,14 +318,20 @@ func parseEnvFile(data []byte) ([]envEntry, error) { if !ok || name == "" { return nil, fmt.Errorf("line %d: use NAME=value", number) } - switch { - case len(value) >= 2 && value[0] == '"' && value[len(value)-1] == '"': - value = strings.ReplaceAll(value[1:len(value)-1], `\n`, "\n") - case len(value) >= 2 && value[0] == '\'' && value[len(value)-1] == '\'': - value = value[1 : len(value)-1] - default: - value, _, _ = strings.Cut(value, " #") + if quote := value[:min(len(value), 1)]; quote == `"` || quote == "'" { + // The value ends at the closing quote; only a # comment may follow it. + closing := strings.Index(value[1:], quote) + 1 + rest := strings.TrimSpace(value[min(closing+1, len(value)):]) + if closing > 0 && (rest == "" || strings.HasPrefix(rest, "#")) { + value = value[1:closing] + if quote == `"` { + value = strings.ReplaceAll(value, `\n`, "\n") + } + entries = append(entries, envEntry{name, value}) + continue + } } + value, _, _ = strings.Cut(value, " #") entries = append(entries, envEntry{name, value}) } if len(entries) == 0 { diff --git a/internal/app/action_values_test.go b/internal/app/action_values_test.go index db3b2e2d..0299f747 100644 --- a/internal/app/action_values_test.go +++ b/internal/app/action_values_test.go @@ -6,6 +6,7 @@ package app import ( "context" "encoding/json" + "io" "net/http" "os" "path/filepath" @@ -66,7 +67,7 @@ func TestActionValueSetReadsValuesAndReportsStoredNames(t *testing.T) { []string{"secret", "set", "token", "--body", "x", "--json"}, "", []string{`PUT actions/secrets/token {"data":"x","description":""}`}, - `{"name":"TOKEN","description":""}`, "", + `{"name":"TOKEN","description":"","created_at":""}`, "", }, { "organization", @@ -99,9 +100,9 @@ func TestActionValueSetReadsValuesAndReportsStoredNames(t *testing.T) { server, requests := actionsAPI(t, map[string]string{ "PUT actions/secrets/lower_name": "204 ", "PUT actions/secrets/TOKEN": "204 ", "PUT actions/secrets/token": "204 ", "PUT api/v1/orgs/acme/actions/secrets/TOKEN": "204 ", "PUT api/v1/user/actions/secrets/TOKEN": "204 ", - "GET api/v1/orgs/acme/actions/variables/API_KEY": `404 {}`, "POST api/v1/orgs/acme/actions/variables/API_KEY": "201 ", - "GET api/v1/orgs/acme/actions/variables/region": `404 {}`, "POST api/v1/orgs/acme/actions/variables/region": "201 ", - "GET api/v1/orgs/acme/actions/variables/MODE": `404 {}`, "POST api/v1/orgs/acme/actions/variables/MODE": "201 ", + "GET api/v1/orgs/acme/actions/variables/API_KEY": `404 {"message":"variable not found"}`, "POST api/v1/orgs/acme/actions/variables/API_KEY": "201 ", + "GET api/v1/orgs/acme/actions/variables/region": `404 {"message":"variable not found"}`, "POST api/v1/orgs/acme/actions/variables/region": "201 ", + "GET api/v1/orgs/acme/actions/variables/MODE": `404 {"message":"variable not found"}`, "POST api/v1/orgs/acme/actions/variables/MODE": "201 ", }) deps, out, diagnostics := actionsDeps(t, server.URL, true) deps.Streams.In = strings.NewReader(tc.stdin) @@ -176,3 +177,35 @@ func TestActionValueDeleteConfirmsWithTheScope(t *testing.T) { require.Equal(t, "✓ Deleted Actions secret DEPLOY_TOKEN from alice/project\n", diagnostics.String()) } } + +func TestParseEnvFileStripsQuotesBeforeAComment(t *testing.T) { + entries, err := parseEnvFile([]byte("A=\"s3cr3t\" # prod\nB='v' # c\nC=\"a # b\"\nD=plain # note\nE=\"x\\ny\"\nF=\"half\n")) + require.NoError(t, err) + require.Equal(t, []envEntry{{"A", "s3cr3t"}, {"B", "v"}, {"C", "a # b"}, {"D", "plain"}, {"E", "x\ny"}, {"F", "\"half"}}, entries) +} + +func TestSecretSetChecksTheNameBeforeReadingTheValue(t *testing.T) { + deps, _, _ := actionsDeps(t, "http://127.0.0.1:1", true) + read := &stdinReads{} + deps.Streams.In = read + err := Execute(context.Background(), deps, []string{"secret", "set", "GITHUB_PAT", "-R", "alice/project"}) + require.ErrorContains(t, err, "reserved") + require.Zero(t, read.reads, "the value must not be read for a refused name") +} + +type stdinReads struct{ reads int } + +func (r *stdinReads) Read([]byte) (int, error) { r.reads++; return 0, io.EOF } + +func TestVariableSetCountsCharactersNotBytes(t *testing.T) { + server, requests := actionsAPI(t, map[string]string{ + "GET api/v1/orgs/acme/actions/variables/WIDE": `404 {"message":"variable not found"}`, "POST api/v1/orgs/acme/actions/variables/WIDE": "201 ", + }) + deps, _, _ := actionsDeps(t, server.URL, true) + deps.Streams.In = strings.NewReader(strings.Repeat("é", 40000)) + require.NoError(t, Execute(context.Background(), deps, []string{"variable", "set", "WIDE", "-o", "acme"})) + require.Len(t, *requests, 2) + deps, _, _ = actionsDeps(t, server.URL, true) + deps.Streams.In = strings.NewReader(strings.Repeat("é", 65537)) + require.ErrorContains(t, Execute(context.Background(), deps, []string{"variable", "set", "WIDE", "-o", "acme"}), "65536 characters") +} diff --git a/internal/app/alias_completion_test.go b/internal/app/alias_completion_test.go index 0f5d3ccf..78f8b39a 100644 --- a/internal/app/alias_completion_test.go +++ b/internal/app/alias_completion_test.go @@ -347,3 +347,33 @@ func TestAccountCompletionUsesInstanceOfRepository(t *testing.T) { require.Equal(t, []string{"adam\thttps://a.example.org", "alice\thttps://a.example.org (active)"}, values, command) } } + +func TestActionValueCompletionOutsideARepository(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/orgs/acme/actions/variables": + _, _ = w.Write([]byte(`[{"name":"REGION","data":"eu"}]`)) + case "/api/v1/user/actions/variables": + _, _ = w.Write([]byte(`[{"name":"HOME_REGION","data":"eu"}]`)) + default: + t.Errorf("unexpected request %s", r.URL.Path) + w.WriteHeader(http.StatusTeapot) + } + })) + defer server.Close() + deps, _, _ := serverDeps(t, server.URL) + a := &application{deps: deps} + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + for name, tc := range map[string]struct { + scope services.ActionScope + want []string + }{ + "organization": {services.ActionScope{Organization: "acme"}, []string{"REGION"}}, + "account": {services.ActionScope{Account: true}, []string{"HOME_REGION"}}, + "repository": {services.ActionScope{}, nil}, + } { + values, _ := a.completeVariables(func() services.ActionScope { return tc.scope })(cmd, nil, "") + require.ElementsMatch(t, tc.want, values, name) + } +} diff --git a/internal/app/completion.go b/internal/app/completion.go index e931290d..98ea4cfd 100644 --- a/internal/app/completion.go +++ b/internal/app/completion.go @@ -490,16 +490,24 @@ func (a *application) completeRuns() cobra.CompletionFunc { // completeSecrets completes Actions secret names in the selected scope. func (a *application) completeSecrets(scope func() services.ActionScope) cobra.CompletionFunc { - return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - items, err := services.ActionSecrets{Client: t.client, Scope: actionScope(scope, t)}.ListPage(ctx, completionLimit, 1) + return a.apiCompletion(false, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { + selected := actionScope(scope, t) + if selected.Repository == "" && selected.Organization == "" && !selected.Account { + return nil, nil + } + items, err := services.ActionSecrets{Client: t.client, Scope: selected}.ListPage(ctx, completionLimit, 1) return describe(items.Items, func(item services.ActionSecret) (string, string) { return item.Name, item.Description }), err }) } // completeVariables completes Actions variable names in the selected scope. func (a *application) completeVariables(scope func() services.ActionScope) cobra.CompletionFunc { - return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - items, err := services.ActionVariables{Client: t.client, Scope: actionScope(scope, t)}.ListPage(ctx, completionLimit, 1) + return a.apiCompletion(false, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { + selected := actionScope(scope, t) + if selected.Repository == "" && selected.Organization == "" && !selected.Account { + return nil, nil + } + items, err := services.ActionVariables{Client: t.client, Scope: selected}.ListPage(ctx, completionLimit, 1) return describe(items.Items, func(item services.ActionVariable) (string, string) { return item.Name, item.Description }), err }) } diff --git a/internal/app/validation_test.go b/internal/app/validation_test.go index 20ed89a1..3b9dd8d3 100644 --- a/internal/app/validation_test.go +++ b/internal/app/validation_test.go @@ -294,6 +294,7 @@ func TestPromptedCreateSubmitsWithoutConfirmation(t *testing.T) { server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"variable not found"}`)) return } writes = append(writes, r.Method+" "+r.URL.Path) diff --git a/internal/services/action_values.go b/internal/services/action_values.go index b3cd1fef..b40100fe 100644 --- a/internal/services/action_values.go +++ b/internal/services/action_values.go @@ -36,12 +36,21 @@ func (s ActionScope) String() string { type ActionSecret struct { Name string `json:"name"` Description string `json:"description"` - CreatedAt string `json:"created_at,omitempty"` + CreatedAt string `json:"created_at"` } // ActionValueName is the name Gitea stores a secret or variable under: names are upper-cased. func ActionValueName(name string) string { return strings.ToUpper(name) } +// classifyValue classifies err as a missing secret or variable only when the 404 says so: Gitea +// answers a plain "not found" when the repository or organization itself does not exist. +func classifyValue(err error, kind, name string, scope ActionScope) error { + if apiErr, ok := errors.AsType[*transport.APIError](err); ok && apiErr.Status == http.StatusNotFound && !strings.Contains(apiErr.Message, kind) { + return err + } + return Classify(err, kind, ActionValueName(name), scope.String()) +} + func secretFrom(secret *gitea.Secret) ActionSecret { return ActionSecret{Name: secret.Name, Description: secret.Description, CreatedAt: timestamp(secret.Created)} } @@ -117,7 +126,7 @@ func (s ActionSecrets) Delete(ctx context.Context, name string) error { // TODO(sdk): DeleteRepoSecret does not escape the secret name. return c.Actions.DeleteRepoSecret(ctx, owner, repo, url.PathEscape(name)) }) - return Classify(err, "secret", ActionValueName(name), s.Scope.String()) + return classifyValue(err, "secret", name, s.Scope) } // ActionVariables manages scoped non-secret Actions variables. @@ -171,23 +180,28 @@ func (s ActionVariables) View(ctx context.Context, name string) (ActionVariable, err = getJSON(ctx, s.Client, s.repoVariables(name), variable) } if err != nil { - return ActionVariable{}, Classify(err, "variable", ActionValueName(name), s.Scope.String()) + return ActionVariable{}, classifyValue(err, "variable", name, s.Scope) } return variableFrom(variable), nil } -// Set chooses create or update from an authoritative read. -func (s ActionVariables) Set(ctx context.Context, name, value, description string) (ActionVariable, error) { +// Set chooses create or update from an authoritative read. A nil description keeps the one of an +// existing variable, as Gitea replaces it on every update. +func (s ActionVariables) Set(ctx context.Context, name, value string, newDescription *string) (ActionVariable, error) { exists := true - if _, err := s.View(ctx, name); err != nil { + current, err := s.View(ctx, name) + if err != nil { if !errors.As(err, new(*NotFoundError)) { return ActionVariable{}, err } exists = false } + description := current.Description + if newDescription != nil { + description = *newDescription + } create := gitea.CreateActionsVariableOption{Value: value, Description: description} update := gitea.UpdateActionsVariableOption{Name: name, Value: value, Description: description} - var err error switch { case s.Scope.Account: err = do(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) { @@ -228,5 +242,5 @@ func (s ActionVariables) Delete(ctx context.Context, name string) error { default: err = sendJSON(ctx, s.Client, http.MethodDelete, s.repoVariables(name), nil, nil) } - return Classify(err, "variable", ActionValueName(name), s.Scope.String()) + return classifyValue(err, "variable", name, s.Scope) } diff --git a/internal/services/action_values_test.go b/internal/services/action_values_test.go index 53235354..6f73d7f4 100644 --- a/internal/services/action_values_test.go +++ b/internal/services/action_values_test.go @@ -6,8 +6,10 @@ package services import ( "context" "encoding/json" + "errors" "fmt" "net/http" + "strings" "testing" "gitea.dev/tea/internal/transport" @@ -37,7 +39,7 @@ func TestVariableSetSelectsCreateOrUpdate(t *testing.T) { assert.Equal(t, scope.path+"/variables/BUILD_MODE", r.URL.Path) if r.Method == http.MethodGet { w.WriteHeader(test.read) - _, _ = w.Write([]byte(`{"name":"BUILD_MODE","data":"old"}`)) + _, _ = w.Write([]byte(`{"name":"BUILD_MODE","data":"old","message":"variable not found"}`)) return } mutations = append(mutations, r.Method) @@ -47,7 +49,7 @@ func TestVariableSetSelectsCreateOrUpdate(t *testing.T) { assert.Equal(t, "build flavor", payload["description"]) w.WriteHeader(http.StatusNoContent) })) - item, err := (ActionVariables{Client: client, Scope: scope.scope}).Set(context.Background(), "BUILD_MODE", "new\nvalue", "build flavor") + item, err := (ActionVariables{Client: client, Scope: scope.scope}).Set(context.Background(), "BUILD_MODE", "new\nvalue", new("build flavor")) if test.wantMethod == "" { var apiErr *transport.APIError require.ErrorAs(t, err, &apiErr) @@ -139,3 +141,62 @@ func TestSecretListReturnsMetadataOnly(t *testing.T) { }) } } + +func TestVariableSetKeepsTheDescriptionUnlessGiven(t *testing.T) { + for _, test := range []struct { + name string + description *string + want string + }{{"omitted", nil, "build flavor"}, {"given", new("other"), "other"}, {"cleared", new(""), ""}} { + t.Run(test.name, func(t *testing.T) { + var sent string + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"name":"MODE","data":"old","description":"build flavor"}`)) + return + } + var payload map[string]string + assert.NoError(t, json.NewDecoder(r.Body).Decode(&payload)) + sent = payload["description"] + w.WriteHeader(http.StatusNoContent) + })) + item, err := (ActionVariables{Client: client, Scope: ActionScope{Organization: "team"}}).Set(context.Background(), "MODE", "new", test.description) + require.NoError(t, err) + require.Equal(t, test.want, sent) + require.Equal(t, test.want, item.Description) + }) + } +} + +func TestMissingScopeIsNotReportedAsAMissingValue(t *testing.T) { + for _, test := range []struct { + message string + missing bool + }{{"not found", false}, {"secret was not found [name: ]", true}, {"variable not found", true}} { + t.Run(test.message, func(t *testing.T) { + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"` + test.message + `"}`)) + })) + scope := ActionScope{Organization: "acme"} + for name, err := range map[string]error{ + "secret": (ActionSecrets{Client: client, Scope: scope}).Delete(context.Background(), "TOKEN"), + "variable": (ActionVariables{Client: client, Scope: scope}).Delete(context.Background(), "MODE"), + } { + var missing *NotFoundError + require.Error(t, err) + if strings.Contains(test.message, name) { + require.ErrorAs(t, err, &missing, name) + } else { + require.False(t, errors.As(err, &missing), name) + } + } + }) + } +} + +func TestSecretSetResultHasEveryJSONField(t *testing.T) { + data, err := json.Marshal(ActionSecret{Name: "TOKEN"}) + require.NoError(t, err) + require.JSONEq(t, `{"name":"TOKEN","description":"","created_at":""}`, string(data)) +} diff --git a/internal/services/actions.go b/internal/services/actions.go index ef79f747..e53ecd08 100644 --- a/internal/services/actions.go +++ b/internal/services/actions.go @@ -121,6 +121,12 @@ func (s Actions) Dispatch(ctx context.Context, id, ref string, inputs map[string return c.Actions.DispatchRepoWorkflow(ctx, owner, repo, id, gitea.CreateActionsWorkflowDispatchOption{Ref: ref, Inputs: inputs}, true) }) if api, ok := errors.AsType[*transport.APIError](err); ok && api.Status == http.StatusNotFound && strings.Contains(api.Message, "ref") { + // Gitea resolves a bare ref as a branch only; a tag needs its full name. + if !strings.HasPrefix(ref, "refs/") { + if retried, retryErr := s.Dispatch(ctx, id, "refs/tags/"+ref, inputs); retryErr == nil { + return retried, nil + } + } return DispatchResult{}, &NotFoundError{Kind: "ref", Key: strconv.Quote(ref), Scope: s.Repository, Err: err} } if err != nil { diff --git a/internal/services/actions_test.go b/internal/services/actions_test.go index 3f0b9322..159b7616 100644 --- a/internal/services/actions_test.go +++ b/internal/services/actions_test.go @@ -336,3 +336,29 @@ func TestWorkflowFileTellsAMissingRefFromAMissingFile(t *testing.T) { _, err = service.WorkflowFile(context.Background(), workflow, "main") require.EqualError(t, err, "workflow file .gitea/workflows/ci.yml not found in owner/repo") } + +func TestDispatchFallsBackToATagRef(t *testing.T) { + var refs []string + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var payload struct { + Ref string `json:"ref"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&payload)) + refs = append(refs, payload.Ref) + if payload.Ref == "v1.0" || strings.HasSuffix(payload.Ref, "nope") { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"ref \"` + payload.Ref + `\" not found"}`)) + return + } + _, _ = w.Write([]byte(`{"workflow_run_id":12}`)) + })) + service := Actions{Client: client, Repository: "owner/repo"} + result, err := service.Dispatch(context.Background(), "ci.yml", "v1.0", nil) + require.NoError(t, err) + require.Equal(t, int64(12), result.ID) + _, err = service.Dispatch(context.Background(), "ci.yml", "nope", nil) + var missing *NotFoundError + require.ErrorAs(t, err, &missing) + require.Equal(t, "ref", missing.Kind) + require.Equal(t, []string{"v1.0", "refs/tags/v1.0", "nope", "refs/tags/nope"}, refs) +} diff --git a/main.go b/main.go index 8bbe5630..439e9c19 100644 --- a/main.go +++ b/main.go @@ -14,6 +14,7 @@ import ( "gitea.dev/tea/internal/app" "gitea.dev/tea/internal/clierr" + "gitea.dev/tea/internal/terminal" ) func main() { @@ -46,7 +47,7 @@ func run(ctx context.Context, deps app.Dependencies, args []string) int { fmt.Fprintln(deps.Streams.ErrOut, "Canceled.") reportDraft(deps, err) default: - fmt.Fprintf(deps.Streams.ErrOut, "Error: %v\n", app.Explain(ctx, deps, err)) + fmt.Fprintf(deps.Streams.ErrOut, "Error: %s\n", terminal.SafeText(app.Explain(ctx, deps, err).Error())) } return code } diff --git a/main_test.go b/main_test.go index a6809ffe..5e45a76a 100644 --- a/main_test.go +++ b/main_test.go @@ -6,6 +6,7 @@ package main import ( "bytes" "context" + "encoding/base64" "errors" "net/http" "net/http/httptest" @@ -193,3 +194,35 @@ func TestSecondInterruptStopsACommandBlockedOnStdin(t *testing.T) { t.Fatal("tea ignored the second interrupt while waiting on stdin") } } + +func TestRunDropsTerminalControlsFromErrors(t *testing.T) { + definition := base64.StdEncoding.EncodeToString([]byte("on:\n workflow_dispatch:\n inputs:\n target:\n required: true\n")) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/user": + _, _ = w.Write([]byte(`{"login":"alice"}`)) + case "/api/v1/repos/alice/project": + _, _ = w.Write([]byte(`{"full_name":"alice/project","has_issues":true}`)) + case "/api/v1/repos/alice/project/actions/workflows": + _, _ = w.Write([]byte(`{"workflows":[{"id":"ci.yml","name":"CI\u001b]0;PWNED\u0007","path":".gitea/workflows/ci.yml","state":"active"}]}`)) + case "/api/v1/repos/alice/project/contents/.gitea/workflows/ci.yml": + _, _ = w.Write([]byte(`{"type":"file","encoding":"base64","content":"` + definition + `"}`)) + default: + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + var stderr bytes.Buffer + deps := app.Dependencies{ + Settings: settings.New(t.TempDir()), Credentials: &auth.Store{Dir: t.TempDir()}, Client: transport.New, Prompter: decliningPrompter{}, + Streams: terminal.Streams{In: &bytes.Buffer{}, Out: &bytes.Buffer{}, ErrOut: &stderr}, + Getenv: func(key string) string { + return map[string]string{"GITEA_TOKEN": "token", "TEA_HOST": server.URL}[key] + }, + } + require.Equal(t, 2, run(context.Background(), deps, []string{"workflow", "run", "ci.yml", "--ref", "main", "-R", "alice/project", "--no-input"})) + require.Contains(t, stderr.String(), "missing required input target for CI") + require.NotContains(t, stderr.String(), "\x1b") + require.NotContains(t, stderr.String(), "\x07") +}