fix(client): strip terminal controls from error lines and keep URLs out of errors
- commit
- 77dd5e48386bb67ed26ab0ac6e2c0b47795f8d09
- author
- brice · · 2026-10-10 10:21 UTC
- committer
- brice · · 2026-10-10 11:17 UTC
- parent
- ee9ce2c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +43 -5
M cmd/ermite/cli_watch.go +1 -1
| @@ -144,7 +144,7 @@ func newWatchCmd(env *cliEnv) *cobra.Command { | |||
| 144 | 144 | Topics: topics, | |
| 145 | 145 | Backoff: env.backoff, | |
| 146 | 146 | Sleep: env.sleep, | |
| 147 | - | OnNotice: func(msg string) { noticef(cmd, "%s", msg) }, | |
| 147 | + | OnNotice: func(msg string) { noticef(cmd, "%s", clean(msg)) }, | |
| 148 | 148 | } | |
| 149 | 149 | return c.Watch(ctx, opts, func(ev *ermite.Event) error { | |
| 150 | 150 | e, ok := toJSONEvent(ev) | |
| @@ -144,7 +144,7 @@ func newWatchCmd(env *cliEnv) *cobra.Command { | |||||
| 144 | Topics: topics, | 144 | Topics: topics, | ||
| 145 | Backoff: env.backoff, | 145 | Backoff: env.backoff, | ||
| 146 | Sleep: env.sleep, | 146 | Sleep: env.sleep, | ||
| 147 | - | OnNotice: func(msg string) { noticef(cmd, "%s", msg) }, | 147 | + | OnNotice: func(msg string) { noticef(cmd, "%s", clean(msg)) }, |
| 148 | } | 148 | } | ||
| 149 | return c.Watch(ctx, opts, func(ev *ermite.Event) error { | 149 | return c.Watch(ctx, opts, func(ev *ermite.Event) error { | ||
| 150 | e, ok := toJSONEvent(ev) | 150 | e, ok := toJSONEvent(ev) | ||
M internal/adapters/client/client_test.go +13 -0
| @@ -141,3 +141,16 @@ func TestMissingFeatures(t *testing.T) { | |||
| 141 | 141 | assert.Equal(t, []string{client.FeatureEventsResume}, client.MissingFeatures([]string{client.FeatureRepoUpdate, "other"})) | |
| 142 | 142 | assert.Equal(t, client.UsedFeatures(), client.MissingFeatures(nil)) | |
| 143 | 143 | } | |
| 144 | + | ||
| 145 | + | func TestDescribeStripsTerminalControls(t *testing.T) { | |
| 146 | + | c, err := client.New(client.Config{URL: "http://srv.test", Token: "t"}) | |
| 147 | + | require.NoError(t, err) | |
| 148 | + | msg := "bad\x1b]0;pwn\x07 name \u202egpj.exe \u2066x\u2069" | |
| 149 | + | got := c.Describe(connect.NewError(connect.CodeInvalidArgument, errors.New(msg))).Error() | |
| 150 | + | assert.NotContains(t, got, "\x1b") | |
| 151 | + | assert.NotContains(t, got, "\x07") | |
| 152 | + | assert.NotContains(t, got, "\u202e") | |
| 153 | + | assert.NotContains(t, got, "\u2066") | |
| 154 | + | assert.NotContains(t, got, "\u2069") | |
| 155 | + | assert.Contains(t, got, "bad?]0;pwn? name ?gpj.exe ?x?") | |
| 156 | + | } | |
| @@ -141,3 +141,16 @@ func TestMissingFeatures(t *testing.T) { | |||||
| 141 | assert.Equal(t, []string{client.FeatureEventsResume}, client.MissingFeatures([]string{client.FeatureRepoUpdate, "other"})) | 141 | assert.Equal(t, []string{client.FeatureEventsResume}, client.MissingFeatures([]string{client.FeatureRepoUpdate, "other"})) | ||
| 142 | assert.Equal(t, client.UsedFeatures(), client.MissingFeatures(nil)) | 142 | assert.Equal(t, client.UsedFeatures(), client.MissingFeatures(nil)) | ||
| 143 | } | 143 | } | ||
| 144 | + | ||||
| 145 | + | func TestDescribeStripsTerminalControls(t *testing.T) { | |||
| 146 | + | c, err := client.New(client.Config{URL: "http://srv.test", Token: "t"}) | |||
| 147 | + | require.NoError(t, err) | |||
| 148 | + | msg := "bad\x1b]0;pwn\x07 name \u202egpj.exe \u2066x\u2069" | |||
| 149 | + | got := c.Describe(connect.NewError(connect.CodeInvalidArgument, errors.New(msg))).Error() | |||
| 150 | + | assert.NotContains(t, got, "\x1b") | |||
| 151 | + | assert.NotContains(t, got, "\x07") | |||
| 152 | + | assert.NotContains(t, got, "\u202e") | |||
| 153 | + | assert.NotContains(t, got, "\u2066") | |||
| 154 | + | assert.NotContains(t, got, "\u2069") | |||
| 155 | + | assert.Contains(t, got, "bad?]0;pwn? name ?gpj.exe ?x?") | |||
| 156 | + | } | |||
M internal/adapters/client/config.go +3 -3
| @@ -74,11 +74,11 @@ func ParseURL(raw string) (*url.URL, error) { | |||
| 74 | 74 | case u.User != nil: | |
| 75 | 75 | return nil, errors.New("must not contain credentials: use ERMITE_TOKEN for the token") | |
| 76 | 76 | case u.Scheme != "http" && u.Scheme != "https": | |
| 77 | - | return nil, fmt.Errorf("%q is not an http(s) URL: it must start with http:// or https://", raw) | |
| 77 | + | return nil, errors.New("not an http(s) URL: it must start with http:// or https://") | |
| 78 | 78 | case u.Hostname() == "": | |
| 79 | - | return nil, fmt.Errorf("%q has no host", raw) | |
| 79 | + | return nil, errors.New("the URL has no host") | |
| 80 | 80 | case u.RawQuery != "" || u.ForceQuery || u.Fragment != "": | |
| 81 | - | return nil, fmt.Errorf("%q must not contain a query or a fragment", raw) | |
| 81 | + | return nil, errors.New("the URL must not contain a query or a fragment") | |
| 82 | 82 | } | |
| 83 | 83 | u.Scheme = strings.ToLower(u.Scheme) | |
| 84 | 84 | u.Path = strings.TrimRight(u.Path, "/") | |
| @@ -74,11 +74,11 @@ func ParseURL(raw string) (*url.URL, error) { | |||||
| 74 | case u.User != nil: | 74 | case u.User != nil: | ||
| 75 | return nil, errors.New("must not contain credentials: use ERMITE_TOKEN for the token") | 75 | return nil, errors.New("must not contain credentials: use ERMITE_TOKEN for the token") | ||
| 76 | case u.Scheme != "http" && u.Scheme != "https": | 76 | case u.Scheme != "http" && u.Scheme != "https": | ||
| 77 | - | return nil, fmt.Errorf("%q is not an http(s) URL: it must start with http:// or https://", raw) | 77 | + | return nil, errors.New("not an http(s) URL: it must start with http:// or https://") |
| 78 | case u.Hostname() == "": | 78 | case u.Hostname() == "": | ||
| 79 | - | return nil, fmt.Errorf("%q has no host", raw) | 79 | + | return nil, errors.New("the URL has no host") |
| 80 | case u.RawQuery != "" || u.ForceQuery || u.Fragment != "": | 80 | case u.RawQuery != "" || u.ForceQuery || u.Fragment != "": | ||
| 81 | - | return nil, fmt.Errorf("%q must not contain a query or a fragment", raw) | 81 | + | return nil, errors.New("the URL must not contain a query or a fragment") |
| 82 | } | 82 | } | ||
| 83 | u.Scheme = strings.ToLower(u.Scheme) | 83 | u.Scheme = strings.ToLower(u.Scheme) | ||
| 84 | u.Path = strings.TrimRight(u.Path, "/") | 84 | u.Path = strings.TrimRight(u.Path, "/") | ||
M internal/adapters/client/config_test.go +9 -0
| @@ -67,3 +67,12 @@ func TestInsecureTokenWarning(t *testing.T) { | |||
| 67 | 67 | assert.NotContains(t, w, tt.token+"_", "never echo the token") | |
| 68 | 68 | } | |
| 69 | 69 | } | |
| 70 | + | ||
| 71 | + | func TestParseURLErrorsDoNotEchoInput(t *testing.T) { | |
| 72 | + | for _, raw := range []string{"user:s3cret@host", "s3cret.example.com", "http://", "https://h/?k=s3cret", "http://[s3cret"} { | |
| 73 | + | _, err := client.ParseURL(raw) | |
| 74 | + | require.Error(t, err, raw) | |
| 75 | + | assert.NotContains(t, err.Error(), "s3cret", raw) | |
| 76 | + | assert.NotContains(t, err.Error(), "user:", raw) | |
| 77 | + | } | |
| 78 | + | } | |
| @@ -67,3 +67,12 @@ func TestInsecureTokenWarning(t *testing.T) { | |||||
| 67 | assert.NotContains(t, w, tt.token+"_", "never echo the token") | 67 | assert.NotContains(t, w, tt.token+"_", "never echo the token") | ||
| 68 | } | 68 | } | ||
| 69 | } | 69 | } | ||
| 70 | + | ||||
| 71 | + | func TestParseURLErrorsDoNotEchoInput(t *testing.T) { | |||
| 72 | + | for _, raw := range []string{"user:s3cret@host", "s3cret.example.com", "http://", "https://h/?k=s3cret", "http://[s3cret"} { | |||
| 73 | + | _, err := client.ParseURL(raw) | |||
| 74 | + | require.Error(t, err, raw) | |||
| 75 | + | assert.NotContains(t, err.Error(), "s3cret", raw) | |||
| 76 | + | assert.NotContains(t, err.Error(), "user:", raw) | |||
| 77 | + | } | |||
| 78 | + | } | |||
M internal/adapters/client/errors.go +17 -1
| @@ -93,9 +93,25 @@ func (c *Client) Describe(err error) error { | |||
| 93 | 93 | return &Error{msg: text, err: err} | |
| 94 | 94 | } | |
| 95 | 95 | ||
| 96 | + | // Clean makes a server-provided string safe for a terminal: line breaks and | |
| 97 | + | // tabs become spaces; other control characters (escape sequences) and bidi | |
| 98 | + | // override or isolate characters are replaced by "?". | |
| 99 | + | func Clean(s string) string { | |
| 100 | + | return strings.Map(func(r rune) rune { | |
| 101 | + | switch { | |
| 102 | + | case r == '\t' || r == '\n' || r == '\r': | |
| 103 | + | return ' ' | |
| 104 | + | case r < 0x20 || r == 0x7f || (r >= 0x80 && r < 0xa0), | |
| 105 | + | r >= 0x202a && r <= 0x202e, r >= 0x2066 && r <= 0x2069: | |
| 106 | + | return '?' | |
| 107 | + | } | |
| 108 | + | return r | |
| 109 | + | }, s) | |
| 110 | + | } | |
| 111 | + | ||
| 96 | 112 | // oneLine collapses whitespace and bounds the length of a server message. | |
| 97 | 113 | func oneLine(s string) string { | |
| 98 | - | s = strings.Join(strings.Fields(s), " ") | |
| 114 | + | s = strings.Join(strings.Fields(Clean(s)), " ") | |
| 99 | 115 | if len(s) > maxMessage { | |
| 100 | 116 | s = strings.ToValidUTF8(s[:maxMessage], "") + "…" | |
| 101 | 117 | } | |
| @@ -93,9 +93,25 @@ func (c *Client) Describe(err error) error { | |||||
| 93 | return &Error{msg: text, err: err} | 93 | return &Error{msg: text, err: err} | ||
| 94 | } | 94 | } | ||
| 95 | 95 | ||||
| 96 | + | // Clean makes a server-provided string safe for a terminal: line breaks and | |||
| 97 | + | // tabs become spaces; other control characters (escape sequences) and bidi | |||
| 98 | + | // override or isolate characters are replaced by "?". | |||
| 99 | + | func Clean(s string) string { | |||
| 100 | + | return strings.Map(func(r rune) rune { | |||
| 101 | + | switch { | |||
| 102 | + | case r == '\t' || r == '\n' || r == '\r': | |||
| 103 | + | return ' ' | |||
| 104 | + | case r < 0x20 || r == 0x7f || (r >= 0x80 && r < 0xa0), | |||
| 105 | + | r >= 0x202a && r <= 0x202e, r >= 0x2066 && r <= 0x2069: | |||
| 106 | + | return '?' | |||
| 107 | + | } | |||
| 108 | + | return r | |||
| 109 | + | }, s) | |||
| 110 | + | } | |||
| 111 | + | ||||
| 96 | // oneLine collapses whitespace and bounds the length of a server message. | 112 | // oneLine collapses whitespace and bounds the length of a server message. | ||
| 97 | func oneLine(s string) string { | 113 | func oneLine(s string) string { | ||
| 98 | - | s = strings.Join(strings.Fields(s), " ") | 114 | + | s = strings.Join(strings.Fields(Clean(s)), " ") |
| 99 | if len(s) > maxMessage { | 115 | if len(s) > maxMessage { | ||
| 100 | s = strings.ToValidUTF8(s[:maxMessage], "") + "…" | 116 | s = strings.ToValidUTF8(s[:maxMessage], "") + "…" | ||
| 101 | } | 117 | } | ||