From 33f3ff10b6dba5170f25f187315ac885b6f1a3c1 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 18 Oct 2026 20:56:34 +0200 Subject: [PATCH] fix: drop only control character references from Markdown, keep escaped quotes in env files, reject prefill-less inline comment edits, relabel the token's account Signed-off-by: bircni --- internal/app/action_values.go | 18 ++++++++++++-- internal/app/action_values_test.go | 7 ++++++ internal/app/comments.go | 4 +++ internal/app/editor_test.go | 2 +- internal/app/target.go | 2 +- internal/terminal/markdown.go | 39 ++++++++++++++++++++++-------- internal/terminal/view_test.go | 7 ++++++ 7 files changed, 65 insertions(+), 14 deletions(-) diff --git a/internal/app/action_values.go b/internal/app/action_values.go index 405e30ec..9ae754f5 100644 --- a/internal/app/action_values.go +++ b/internal/app/action_values.go @@ -320,12 +320,12 @@ func parseEnvFile(data []byte) ([]envEntry, error) { } 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 + closing := closingQuote(value, quote) 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") + value = strings.NewReplacer(`\n`, "\n", `\"`, `"`, `\\`, `\`).Replace(value) } entries = append(entries, envEntry{name, value}) continue @@ -340,6 +340,20 @@ func parseEnvFile(data []byte) ([]envEntry, error) { return entries, scanner.Err() } +// closingQuote returns the index of the quote that closes the quoted value, or 0 when it is not +// closed. Inside double quotes a backslash escapes the next character. +func closingQuote(value, quote string) int { + for i := 1; i < len(value); i++ { + switch { + case quote == `"` && value[i] == '\\': + i++ + case string(value[i]) == quote: + return i + } + } + return 0 +} + func (a *application) actionValueDeleteCommand(secret bool, kind string) *cobra.Command { var scope actionScopeFlags cmd := &cobra.Command{ diff --git a/internal/app/action_values_test.go b/internal/app/action_values_test.go index 0299f747..ca4a1b42 100644 --- a/internal/app/action_values_test.go +++ b/internal/app/action_values_test.go @@ -209,3 +209,10 @@ func TestVariableSetCountsCharactersNotBytes(t *testing.T) { deps.Streams.In = strings.NewReader(strings.Repeat("é", 65537)) require.ErrorContains(t, Execute(context.Background(), deps, []string{"variable", "set", "WIDE", "-o", "acme"}), "65536 characters") } + +func TestParseEnvFileKeepsEscapedQuotes(t *testing.T) { + entries, err := parseEnvFile([]byte("A=\"say \\\"hi\\\" now\" # note\nB='it''s'\nC=\"back\\\\slash\\nline\"\n")) + require.NoError(t, err) + require.Equal(t, "say \"hi\" now", entries[0].value) + require.Equal(t, "back\\slash\nline", entries[2].value) +} diff --git a/internal/app/comments.go b/internal/app/comments.go index 2e37520e..4f298077 100644 --- a/internal/app/comments.go +++ b/internal/app/comments.go @@ -187,6 +187,10 @@ func (a *application) comment(cmd *cobra.Command, arg string, kind refKind, chan // editComment replaces the text of comment, prefilling the prompt or editor with it. func (a *application) editComment(cmd *cobra.Command, t target, comment services.Comment, f commentFlags) (runErr error) { + // An inline review comment is not fetched, so there is no text to start from. + if comment.Author == "" && !cmd.Flags().Changed("body") && !cmd.Flags().Changed("body-file") { + return clierr.Wrap(clierr.Usage, errors.New("an inline review comment cannot be prefilled; supply --body or --body-file")) + } body, draft, err := a.readComment(cmd, f, comment.Body) defer finishDraft(&draft, &runErr) if err != nil { diff --git a/internal/app/editor_test.go b/internal/app/editor_test.go index 1ee6fb8e..ad6bdb8c 100644 --- a/internal/app/editor_test.go +++ b/internal/app/editor_test.go @@ -68,7 +68,7 @@ func TestCommentEditEditorSeedsCurrentBody(t *testing.T) { server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project/issues/comments/7": - _, _ = w.Write([]byte(`{"id":7,"body":"current comment"}`)) + _, _ = w.Write([]byte(`{"id":7,"body":"current comment","user":{"login":"alice"}}`)) case r.Method == http.MethodPatch && r.URL.Path == "/api/v1/repos/alice/project/issues/comments/7": var options struct{ Body string } require.NoError(t, json.NewDecoder(r.Body).Decode(&options)) diff --git a/internal/app/target.go b/internal/app/target.go index b0a6ffad..de9592e9 100644 --- a/internal/app/target.go +++ b/internal/app/target.go @@ -192,7 +192,7 @@ func (a *application) connect(ctx context.Context, req targetRequest) (target, e if t.explicitUser() && !services.MatchAccount(t.User, actual) { return t, clierr.Wrap(clierr.Authentication, errEnvironmentAccount) } - if t.User == "" { + if !t.explicitUser() { t.UserSource = "GITEA_TOKEN authenticated account" } t.User = actual diff --git a/internal/terminal/markdown.go b/internal/terminal/markdown.go index 17bc8513..28514ad5 100644 --- a/internal/terminal/markdown.go +++ b/internal/terminal/markdown.go @@ -4,10 +4,11 @@ package terminal import ( - "html" "regexp" + "strconv" "strings" "sync" + "unicode" "unicode/utf8" "charm.land/glamour/v2" @@ -22,6 +23,29 @@ func MarkdownEnabled(preference string, outTTY bool) bool { // glamourLinks matches the OSC 8 hyperlinks that glamour writes for markdown links. var glamourLinks = regexp.MustCompile("\x1b\\]8;[^\x07\x1b]*(?:\x07|\x1b\\\\)") +// numericReference matches a decimal or hexadecimal character reference, with or without the semicolon. +var numericReference = regexp.MustCompile(`&#([0-9]{1,8}|[xX][0-9a-fA-F]{1,8});?`) + +// dropControlReference returns "" for a character reference that names a control or format character +// other than a tab or line break, and the reference itself otherwise. +func dropControlReference(reference string) string { + digits := strings.TrimSuffix(strings.TrimPrefix(strings.TrimPrefix(reference, "&#"), "x"), ";") + digits = strings.TrimPrefix(digits, "X") + base := 10 + if strings.HasPrefix(reference, "&#x") || strings.HasPrefix(reference, "&#X") { + base = 16 + } + n, err := strconv.ParseInt(digits, base, 32) + if err != nil { + return reference + } + r := rune(n) + if r != '\t' && r != '\n' && (r == 0 || unicode.IsControl(r) || unicode.Is(unicode.Cf, r)) { + return "" + } + return reference +} + // glamourSequences matches the escape sequences glamour writes itself: colors and OSC 8 hyperlinks. var glamourSequences = regexp.MustCompile("\x1b\\[[0-9;]*m|\x1b\\]8;[^\x07\x1b]*(?:\x07|\x1b\\\\)") @@ -76,15 +100,10 @@ func markdownRenderer(width int, color bool) (*glamour.TermRenderer, error) { // with links only when d allows them. It keeps line breaks like Gitea does, and returns the sanitized // text unrendered if rendering fails. func Markdown(text string, d Display) string { - // Markdown decodes character references such as  into the characters they name, so decode - // them (to a fixed point, for &#27;) before sanitizing instead of after. - for range 4 { - decoded := html.UnescapeString(text) - if decoded == text { - break - } - text = decoded - } + // Markdown decodes numeric character references such as  into the characters they name, + // which SafeText cannot see, so drop the ones that name a control character before rendering. + // Other entities (&, §ion=...) are left to the renderer so ordinary text is unchanged. + text = numericReference.ReplaceAllStringFunc(text, dropControlReference) text = SafeText(text) width := d.Width if width <= 0 { diff --git a/internal/terminal/view_test.go b/internal/terminal/view_test.go index 468076c3..772f4299 100644 --- a/internal/terminal/view_test.go +++ b/internal/terminal/view_test.go @@ -129,3 +129,10 @@ func TestMarkdownDropsEscapesFromCharacterReferences(t *testing.T) { } } } + +func TestMarkdownKeepsOrdinaryCharacterReferences(t *testing.T) { + plain := Display{Width: 80} + require.Contains(t, Markdown("5 < 6 & © ☺ ©", plain), "5 < 6 & © ☺ ©") + require.Contains(t, Markdown("a b", plain), "a\tb") + require.Equal(t, "ab", strings.TrimSpace(Markdown("a›‮b", plain))) +}