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 <bircni@icloud.com>

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{

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)
+}

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 {

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))

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

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
+	// Markdown decodes numeric character references such as  into the characters they name,
-	// them (to a fixed point, for ) before sanitizing instead of after.
+	// which SafeText cannot see, so drop the ones that name a control character before rendering.
-	for range 4 {
+	// Other entities (&, &section=...) are left to the renderer so ordinary text is unchanged.
-		decoded := html.UnescapeString(text)
+	text = numericReference.ReplaceAllStringFunc(text, dropControlReference)
-		if decoded == text {
-			break
-		}
-		text = decoded
-	}
 	text = SafeText(text)
 	width := d.Width
 	if width <= 0 {

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 &lt; 6 &amp; &#169; &#x263a; &copy;", plain), "5 < 6 & © ☺ ©")
+	require.Contains(t, Markdown("a&#9;b", plain), "a\tb")
+	require.Equal(t, "ab", strings.TrimSpace(Markdown("a&#27;&#x1B;&#7;&#155;&#8238;b", plain)))
+}