fix(remote): bound and sanitize catalog diagnostics
Signed-off-by: brice <b@bnema.dev>
- commit
- b71ae3029b9105372f2c433410518977092407c6
- author
- brice · · 2026-07-29 21:52 UTC
- committer
- brice · · 2026-07-30 09:15 UTC
- parent
- 3644a55
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +229 -8
M internal/adapters/remote/catalog.go +74 -8
| @@ -10,13 +10,18 @@ import ( | |||
| 10 | 10 | "log/slog" | |
| 11 | 11 | "os/exec" | |
| 12 | 12 | "strings" | |
| 13 | + | "unicode" | |
| 14 | + | "unicode/utf8" | |
| 13 | 15 | ||
| 14 | 16 | "github.com/bnema/vev/internal/adapters/sshstdio" | |
| 15 | 17 | "github.com/bnema/vev/internal/domain" | |
| 16 | 18 | "github.com/bnema/vev/internal/ports" | |
| 17 | 19 | ) | |
| 18 | 20 | ||
| 19 | - | const maxCatalogBytes = 1 << 20 | |
| 21 | + | const ( | |
| 22 | + | maxCatalogBytes = 1 << 20 | |
| 23 | + | maxCatalogDiagnosticBytes = 4 << 10 | |
| 24 | + | ) | |
| 20 | 25 | ||
| 21 | 26 | var ( | |
| 22 | 27 | errCatalogDecode = errors.New("remote catalog: invalid JSON") | |
| @@ -56,23 +61,27 @@ func (c *CatalogClient) List(ctx context.Context, target string) (ports.RemoteCa | |||
| 56 | 61 | } | |
| 57 | 62 | spec := sshstdio.BuildCommandForRemoteCommand(target, "vev", "cmd", "remote-catalog", "--json") | |
| 58 | 63 | cmd := command(ctx, spec.Path, spec.Args...) | |
| 59 | - | var stdout boundedBuffer | |
| 60 | - | var stderr bytes.Buffer | |
| 64 | + | stdout := boundedBuffer{limit: maxCatalogBytes} | |
| 65 | + | stderr := boundedBuffer{limit: maxCatalogDiagnosticBytes} | |
| 61 | 66 | cmd.Stdout = &stdout | |
| 62 | 67 | cmd.Stderr = &stderr | |
| 63 | 68 | if err := cmd.Run(); err != nil { | |
| 64 | 69 | if ctxErr := ctx.Err(); ctxErr != nil { | |
| 65 | 70 | return ports.RemoteCatalog{}, ctxErr | |
| 66 | 71 | } | |
| 67 | - | stderrText := strings.TrimSpace(stderr.String()) | |
| 72 | + | if stdout.overflow || stderr.overflow { | |
| 73 | + | slog.Debug("remote catalog output too large", "target", target, "stdout_limit", maxCatalogBytes, "stderr_limit", maxCatalogDiagnosticBytes) | |
| 74 | + | return ports.RemoteCatalog{}, errCatalogTooLarge | |
| 75 | + | } | |
| 76 | + | stderrText := sanitizeCatalogDiagnostic(string(stderr.Bytes())) | |
| 68 | 77 | slog.Debug("remote catalog ssh failed", "target", target, "err", err, "stderr", stderrText) | |
| 69 | 78 | if stderrText != "" { | |
| 70 | 79 | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w: %s", errCatalogSSH, err, stderrText) | |
| 71 | 80 | } | |
| 72 | 81 | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w", errCatalogSSH, err) | |
| 73 | 82 | } | |
| 74 | - | if stdout.overflow { | |
| 75 | - | slog.Debug("remote catalog output too large", "target", target, "limit", maxCatalogBytes) | |
| 83 | + | if stdout.overflow || stderr.overflow { | |
| 84 | + | slog.Debug("remote catalog output too large", "target", target, "stdout_limit", maxCatalogBytes, "stderr_limit", maxCatalogDiagnosticBytes) | |
| 76 | 85 | return ports.RemoteCatalog{}, errCatalogTooLarge | |
| 77 | 86 | } | |
| 78 | 87 | ||
| @@ -149,9 +158,66 @@ func validCatalogState(state string) bool { | |||
| 149 | 158 | } | |
| 150 | 159 | } | |
| 151 | 160 | ||
| 152 | - | // boundedBuffer captures stdout up to maxCatalogBytes and records overflow. | |
| 161 | + | // sanitizeCatalogDiagnostic strips terminal escapes and control bytes while | |
| 162 | + | // preserving useful printable diagnostic text for errors and logs. | |
| 163 | + | func sanitizeCatalogDiagnostic(raw string) string { | |
| 164 | + | var b strings.Builder | |
| 165 | + | b.Grow(len(raw)) | |
| 166 | + | for i := 0; i < len(raw); { | |
| 167 | + | if raw[i] == 0x1b { | |
| 168 | + | i = skipTerminalEscape(raw, i+1) | |
| 169 | + | continue | |
| 170 | + | } | |
| 171 | + | r, size := utf8.DecodeRuneInString(raw[i:]) | |
| 172 | + | if r == utf8.RuneError && size == 1 { | |
| 173 | + | i++ | |
| 174 | + | continue | |
| 175 | + | } | |
| 176 | + | if unicode.IsPrint(r) || r == '\t' || r == '\n' { | |
| 177 | + | b.WriteRune(r) | |
| 178 | + | } | |
| 179 | + | i += size | |
| 180 | + | } | |
| 181 | + | return strings.TrimSpace(b.String()) | |
| 182 | + | } | |
| 183 | + | ||
| 184 | + | func skipTerminalEscape(s string, i int) int { | |
| 185 | + | if i >= len(s) { | |
| 186 | + | return i | |
| 187 | + | } | |
| 188 | + | switch s[i] { | |
| 189 | + | case '[': // CSI: ESC [ ... final byte in 0x40-0x7E | |
| 190 | + | i++ | |
| 191 | + | for i < len(s) { | |
| 192 | + | ch := s[i] | |
| 193 | + | i++ | |
| 194 | + | if ch >= 0x40 && ch <= 0x7e { | |
| 195 | + | break | |
| 196 | + | } | |
| 197 | + | } | |
| 198 | + | return i | |
| 199 | + | case ']': // OSC: ESC ] ... BEL or ST (ESC \) | |
| 200 | + | i++ | |
| 201 | + | for i < len(s) { | |
| 202 | + | if s[i] == 0x07 { | |
| 203 | + | return i + 1 | |
| 204 | + | } | |
| 205 | + | if s[i] == 0x1b && i+1 < len(s) && s[i+1] == '\\' { | |
| 206 | + | return i + 2 | |
| 207 | + | } | |
| 208 | + | i++ | |
| 209 | + | } | |
| 210 | + | return i | |
| 211 | + | default: | |
| 212 | + | // Two-byte Fe escape or unknown: drop ESC and the next byte. | |
| 213 | + | return i + 1 | |
| 214 | + | } | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | // boundedBuffer captures stream output up to an explicit byte limit and records overflow. | |
| 153 | 218 | type boundedBuffer struct { | |
| 154 | 219 | buf bytes.Buffer | |
| 220 | + | limit int | |
| 155 | 221 | overflow bool | |
| 156 | 222 | } | |
| 157 | 223 | ||
| @@ -159,7 +225,7 @@ func (b *boundedBuffer) Write(p []byte) (int, error) { | |||
| 159 | 225 | if b.overflow { | |
| 160 | 226 | return len(p), nil | |
| 161 | 227 | } | |
| 162 | - | remaining := maxCatalogBytes - b.buf.Len() | |
| 228 | + | remaining := b.limit - b.buf.Len() | |
| 163 | 229 | if remaining <= 0 { | |
| 164 | 230 | b.overflow = true | |
| 165 | 231 | return len(p), nil | |
| @@ -10,13 +10,18 @@ import ( | |||||
| 10 | "log/slog" | 10 | "log/slog" | ||
| 11 | "os/exec" | 11 | "os/exec" | ||
| 12 | "strings" | 12 | "strings" | ||
| 13 | + | "unicode" | |||
| 14 | + | "unicode/utf8" | |||
| 13 | 15 | ||||
| 14 | "github.com/bnema/vev/internal/adapters/sshstdio" | 16 | "github.com/bnema/vev/internal/adapters/sshstdio" | ||
| 15 | "github.com/bnema/vev/internal/domain" | 17 | "github.com/bnema/vev/internal/domain" | ||
| 16 | "github.com/bnema/vev/internal/ports" | 18 | "github.com/bnema/vev/internal/ports" | ||
| 17 | ) | 19 | ) | ||
| 18 | 20 | ||||
| 19 | - | const maxCatalogBytes = 1 << 20 | 21 | + | const ( |
| 22 | + | maxCatalogBytes = 1 << 20 | |||
| 23 | + | maxCatalogDiagnosticBytes = 4 << 10 | |||
| 24 | + | ) | |||
| 20 | 25 | ||||
| 21 | var ( | 26 | var ( | ||
| 22 | errCatalogDecode = errors.New("remote catalog: invalid JSON") | 27 | errCatalogDecode = errors.New("remote catalog: invalid JSON") | ||
| @@ -56,23 +61,27 @@ func (c *CatalogClient) List(ctx context.Context, target string) (ports.RemoteCa | |||||
| 56 | } | 61 | } | ||
| 57 | spec := sshstdio.BuildCommandForRemoteCommand(target, "vev", "cmd", "remote-catalog", "--json") | 62 | spec := sshstdio.BuildCommandForRemoteCommand(target, "vev", "cmd", "remote-catalog", "--json") | ||
| 58 | cmd := command(ctx, spec.Path, spec.Args...) | 63 | cmd := command(ctx, spec.Path, spec.Args...) | ||
| 59 | - | var stdout boundedBuffer | 64 | + | stdout := boundedBuffer{limit: maxCatalogBytes} |
| 60 | - | var stderr bytes.Buffer | 65 | + | stderr := boundedBuffer{limit: maxCatalogDiagnosticBytes} |
| 61 | cmd.Stdout = &stdout | 66 | cmd.Stdout = &stdout | ||
| 62 | cmd.Stderr = &stderr | 67 | cmd.Stderr = &stderr | ||
| 63 | if err := cmd.Run(); err != nil { | 68 | if err := cmd.Run(); err != nil { | ||
| 64 | if ctxErr := ctx.Err(); ctxErr != nil { | 69 | if ctxErr := ctx.Err(); ctxErr != nil { | ||
| 65 | return ports.RemoteCatalog{}, ctxErr | 70 | return ports.RemoteCatalog{}, ctxErr | ||
| 66 | } | 71 | } | ||
| 67 | - | stderrText := strings.TrimSpace(stderr.String()) | 72 | + | if stdout.overflow || stderr.overflow { |
| 73 | + | slog.Debug("remote catalog output too large", "target", target, "stdout_limit", maxCatalogBytes, "stderr_limit", maxCatalogDiagnosticBytes) | |||
| 74 | + | return ports.RemoteCatalog{}, errCatalogTooLarge | |||
| 75 | + | } | |||
| 76 | + | stderrText := sanitizeCatalogDiagnostic(string(stderr.Bytes())) | |||
| 68 | slog.Debug("remote catalog ssh failed", "target", target, "err", err, "stderr", stderrText) | 77 | slog.Debug("remote catalog ssh failed", "target", target, "err", err, "stderr", stderrText) | ||
| 69 | if stderrText != "" { | 78 | if stderrText != "" { | ||
| 70 | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w: %s", errCatalogSSH, err, stderrText) | 79 | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w: %s", errCatalogSSH, err, stderrText) | ||
| 71 | } | 80 | } | ||
| 72 | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w", errCatalogSSH, err) | 81 | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w", errCatalogSSH, err) | ||
| 73 | } | 82 | } | ||
| 74 | - | if stdout.overflow { | 83 | + | if stdout.overflow || stderr.overflow { |
| 75 | - | slog.Debug("remote catalog output too large", "target", target, "limit", maxCatalogBytes) | 84 | + | slog.Debug("remote catalog output too large", "target", target, "stdout_limit", maxCatalogBytes, "stderr_limit", maxCatalogDiagnosticBytes) |
| 76 | return ports.RemoteCatalog{}, errCatalogTooLarge | 85 | return ports.RemoteCatalog{}, errCatalogTooLarge | ||
| 77 | } | 86 | } | ||
| 78 | 87 | ||||
| @@ -149,9 +158,66 @@ func validCatalogState(state string) bool { | |||||
| 149 | } | 158 | } | ||
| 150 | } | 159 | } | ||
| 151 | 160 | ||||
| 152 | - | // boundedBuffer captures stdout up to maxCatalogBytes and records overflow. | 161 | + | // sanitizeCatalogDiagnostic strips terminal escapes and control bytes while |
| 162 | + | // preserving useful printable diagnostic text for errors and logs. | |||
| 163 | + | func sanitizeCatalogDiagnostic(raw string) string { | |||
| 164 | + | var b strings.Builder | |||
| 165 | + | b.Grow(len(raw)) | |||
| 166 | + | for i := 0; i < len(raw); { | |||
| 167 | + | if raw[i] == 0x1b { | |||
| 168 | + | i = skipTerminalEscape(raw, i+1) | |||
| 169 | + | continue | |||
| 170 | + | } | |||
| 171 | + | r, size := utf8.DecodeRuneInString(raw[i:]) | |||
| 172 | + | if r == utf8.RuneError && size == 1 { | |||
| 173 | + | i++ | |||
| 174 | + | continue | |||
| 175 | + | } | |||
| 176 | + | if unicode.IsPrint(r) || r == '\t' || r == '\n' { | |||
| 177 | + | b.WriteRune(r) | |||
| 178 | + | } | |||
| 179 | + | i += size | |||
| 180 | + | } | |||
| 181 | + | return strings.TrimSpace(b.String()) | |||
| 182 | + | } | |||
| 183 | + | ||||
| 184 | + | func skipTerminalEscape(s string, i int) int { | |||
| 185 | + | if i >= len(s) { | |||
| 186 | + | return i | |||
| 187 | + | } | |||
| 188 | + | switch s[i] { | |||
| 189 | + | case '[': // CSI: ESC [ ... final byte in 0x40-0x7E | |||
| 190 | + | i++ | |||
| 191 | + | for i < len(s) { | |||
| 192 | + | ch := s[i] | |||
| 193 | + | i++ | |||
| 194 | + | if ch >= 0x40 && ch <= 0x7e { | |||
| 195 | + | break | |||
| 196 | + | } | |||
| 197 | + | } | |||
| 198 | + | return i | |||
| 199 | + | case ']': // OSC: ESC ] ... BEL or ST (ESC \) | |||
| 200 | + | i++ | |||
| 201 | + | for i < len(s) { | |||
| 202 | + | if s[i] == 0x07 { | |||
| 203 | + | return i + 1 | |||
| 204 | + | } | |||
| 205 | + | if s[i] == 0x1b && i+1 < len(s) && s[i+1] == '\\' { | |||
| 206 | + | return i + 2 | |||
| 207 | + | } | |||
| 208 | + | i++ | |||
| 209 | + | } | |||
| 210 | + | return i | |||
| 211 | + | default: | |||
| 212 | + | // Two-byte Fe escape or unknown: drop ESC and the next byte. | |||
| 213 | + | return i + 1 | |||
| 214 | + | } | |||
| 215 | + | } | |||
| 216 | + | ||||
| 217 | + | // boundedBuffer captures stream output up to an explicit byte limit and records overflow. | |||
| 153 | type boundedBuffer struct { | 218 | type boundedBuffer struct { | ||
| 154 | buf bytes.Buffer | 219 | buf bytes.Buffer | ||
| 220 | + | limit int | |||
| 155 | overflow bool | 221 | overflow bool | ||
| 156 | } | 222 | } | ||
| 157 | 223 | ||||
| @@ -159,7 +225,7 @@ func (b *boundedBuffer) Write(p []byte) (int, error) { | |||||
| 159 | if b.overflow { | 225 | if b.overflow { | ||
| 160 | return len(p), nil | 226 | return len(p), nil | ||
| 161 | } | 227 | } | ||
| 162 | - | remaining := maxCatalogBytes - b.buf.Len() | 228 | + | remaining := b.limit - b.buf.Len() |
| 163 | if remaining <= 0 { | 229 | if remaining <= 0 { | ||
| 164 | b.overflow = true | 230 | b.overflow = true | ||
| 165 | return len(p), nil | 231 | return len(p), nil | ||
M internal/adapters/remote/catalog_test.go +155 -0
| @@ -220,6 +220,161 @@ func TestRemoteCatalogClientContextCanceled(t *testing.T) { | |||
| 220 | 220 | } | |
| 221 | 221 | } | |
| 222 | 222 | ||
| 223 | + | func TestCatalogOutputSafety(t *testing.T) { | |
| 224 | + | t.Parallel() | |
| 225 | + | ||
| 226 | + | tests := []struct { | |
| 227 | + | name string | |
| 228 | + | command func(ctx context.Context) *exec.Cmd | |
| 229 | + | wantErr error | |
| 230 | + | check func(t *testing.T, err error) | |
| 231 | + | }{ | |
| 232 | + | { | |
| 233 | + | name: "oversized stdout", | |
| 234 | + | command: func(ctx context.Context) *exec.Cmd { | |
| 235 | + | return exec.CommandContext(ctx, "sh", "-c", | |
| 236 | + | fmt.Sprintf("head -c %d /dev/zero", maxCatalogBytes+1)) | |
| 237 | + | }, | |
| 238 | + | wantErr: errCatalogTooLarge, | |
| 239 | + | check: func(t *testing.T, err error) { | |
| 240 | + | t.Helper() | |
| 241 | + | if strings.Contains(err.Error(), strings.Repeat("\x00", 64)) { | |
| 242 | + | t.Fatalf("error embeds oversized stdout: %v", err) | |
| 243 | + | } | |
| 244 | + | }, | |
| 245 | + | }, | |
| 246 | + | { | |
| 247 | + | name: "oversized stderr", | |
| 248 | + | command: func(ctx context.Context) *exec.Cmd { | |
| 249 | + | return exec.CommandContext(ctx, "sh", "-c", | |
| 250 | + | fmt.Sprintf("head -c %d /dev/zero >&2; exit 1", maxCatalogDiagnosticBytes+1)) | |
| 251 | + | }, | |
| 252 | + | wantErr: errCatalogTooLarge, | |
| 253 | + | check: func(t *testing.T, err error) { | |
| 254 | + | t.Helper() | |
| 255 | + | if strings.Contains(err.Error(), strings.Repeat("\x00", 64)) { | |
| 256 | + | t.Fatalf("error embeds oversized stderr: %v", err) | |
| 257 | + | } | |
| 258 | + | if errors.Is(err, errCatalogSSH) { | |
| 259 | + | t.Fatalf("oversized stderr must not wrap SSH with captured output: %v", err) | |
| 260 | + | } | |
| 261 | + | }, | |
| 262 | + | }, | |
| 263 | + | { | |
| 264 | + | name: "ansi control stderr", | |
| 265 | + | command: func(ctx context.Context) *exec.Cmd { | |
| 266 | + | cmd := exec.CommandContext(ctx, "sh", "-c", "cat >&2; exit 255") | |
| 267 | + | cmd.Stdin = strings.NewReader("\x1b[31mconnection refused\x1b[0m\x07") | |
| 268 | + | return cmd | |
| 269 | + | }, | |
| 270 | + | wantErr: errCatalogSSH, | |
| 271 | + | check: func(t *testing.T, err error) { | |
| 272 | + | t.Helper() | |
| 273 | + | msg := err.Error() | |
| 274 | + | if strings.ContainsRune(msg, '\x1b') || strings.ContainsRune(msg, '\x07') { | |
| 275 | + | t.Fatalf("error retains control/ANSI bytes: %q", msg) | |
| 276 | + | } | |
| 277 | + | if strings.Contains(msg, "[31m") || strings.Contains(msg, "[0m") { | |
| 278 | + | t.Fatalf("error retains terminal escape remnants: %q", msg) | |
| 279 | + | } | |
| 280 | + | if !strings.Contains(msg, "connection refused") { | |
| 281 | + | t.Fatalf("error = %q, want printable diagnostic text preserved", msg) | |
| 282 | + | } | |
| 283 | + | }, | |
| 284 | + | }, | |
| 285 | + | } | |
| 286 | + | ||
| 287 | + | for _, tt := range tests { | |
| 288 | + | t.Run(tt.name, func(t *testing.T) { | |
| 289 | + | t.Parallel() | |
| 290 | + | client := &CatalogClient{ | |
| 291 | + | command: func(ctx context.Context, name string, args ...string) *exec.Cmd { | |
| 292 | + | return tt.command(ctx) | |
| 293 | + | }, | |
| 294 | + | } | |
| 295 | + | _, err := client.List(context.Background(), "arch") | |
| 296 | + | if !errors.Is(err, tt.wantErr) { | |
| 297 | + | t.Fatalf("List() error = %v, want %v", err, tt.wantErr) | |
| 298 | + | } | |
| 299 | + | if tt.check != nil { | |
| 300 | + | tt.check(t, err) | |
| 301 | + | } | |
| 302 | + | }) | |
| 303 | + | } | |
| 304 | + | } | |
| 305 | + | ||
| 306 | + | func TestBoundedBufferLimits(t *testing.T) { | |
| 307 | + | t.Parallel() | |
| 308 | + | ||
| 309 | + | tests := []struct { | |
| 310 | + | name string | |
| 311 | + | limit int | |
| 312 | + | chunks [][]byte | |
| 313 | + | wantLen int | |
| 314 | + | wantOver bool | |
| 315 | + | wantBytes string | |
| 316 | + | }{ | |
| 317 | + | { | |
| 318 | + | name: "under limit", | |
| 319 | + | limit: 8, | |
| 320 | + | chunks: [][]byte{[]byte("abcd"), []byte("ef")}, | |
| 321 | + | wantLen: 6, | |
| 322 | + | wantOver: false, | |
| 323 | + | wantBytes: "abcdef", | |
| 324 | + | }, | |
| 325 | + | { | |
| 326 | + | name: "exact limit", | |
| 327 | + | limit: 4, | |
| 328 | + | chunks: [][]byte{[]byte("abcd")}, | |
| 329 | + | wantLen: 4, | |
| 330 | + | wantOver: false, | |
| 331 | + | wantBytes: "abcd", | |
| 332 | + | }, | |
| 333 | + | { | |
| 334 | + | name: "overflow discards remainder", | |
| 335 | + | limit: 4, | |
| 336 | + | chunks: [][]byte{[]byte("abc"), []byte("defgh")}, | |
| 337 | + | wantLen: 4, | |
| 338 | + | wantOver: true, | |
| 339 | + | wantBytes: "abcd", | |
| 340 | + | }, | |
| 341 | + | { | |
| 342 | + | name: "further writes stay discarded", | |
| 343 | + | limit: 2, | |
| 344 | + | chunks: [][]byte{[]byte("abcdef"), []byte("xyz")}, | |
| 345 | + | wantLen: 2, | |
| 346 | + | wantOver: true, | |
| 347 | + | wantBytes: "ab", | |
| 348 | + | }, | |
| 349 | + | } | |
| 350 | + | ||
| 351 | + | for _, tt := range tests { | |
| 352 | + | t.Run(tt.name, func(t *testing.T) { | |
| 353 | + | t.Parallel() | |
| 354 | + | var buf boundedBuffer | |
| 355 | + | buf.limit = tt.limit | |
| 356 | + | for _, chunk := range tt.chunks { | |
| 357 | + | n, err := buf.Write(chunk) | |
| 358 | + | if err != nil { | |
| 359 | + | t.Fatalf("Write(%q) error = %v", chunk, err) | |
| 360 | + | } | |
| 361 | + | if n != len(chunk) { | |
| 362 | + | t.Fatalf("Write(%q) n = %d, want %d", chunk, n, len(chunk)) | |
| 363 | + | } | |
| 364 | + | } | |
| 365 | + | if buf.overflow != tt.wantOver { | |
| 366 | + | t.Fatalf("overflow = %v, want %v", buf.overflow, tt.wantOver) | |
| 367 | + | } | |
| 368 | + | if got := len(buf.Bytes()); got != tt.wantLen { | |
| 369 | + | t.Fatalf("len = %d, want %d", got, tt.wantLen) | |
| 370 | + | } | |
| 371 | + | if got := string(buf.Bytes()); got != tt.wantBytes { | |
| 372 | + | t.Fatalf("bytes = %q, want %q", got, tt.wantBytes) | |
| 373 | + | } | |
| 374 | + | }) | |
| 375 | + | } | |
| 376 | + | } | |
| 377 | + | ||
| 223 | 378 | func stdoutCmd(ctx context.Context, stdout string) *exec.Cmd { | |
| 224 | 379 | cmd := exec.CommandContext(ctx, "cat") | |
| 225 | 380 | cmd.Stdin = strings.NewReader(stdout) | |
| @@ -220,6 +220,161 @@ func TestRemoteCatalogClientContextCanceled(t *testing.T) { | |||||
| 220 | } | 220 | } | ||
| 221 | } | 221 | } | ||
| 222 | 222 | ||||
| 223 | + | func TestCatalogOutputSafety(t *testing.T) { | |||
| 224 | + | t.Parallel() | |||
| 225 | + | ||||
| 226 | + | tests := []struct { | |||
| 227 | + | name string | |||
| 228 | + | command func(ctx context.Context) *exec.Cmd | |||
| 229 | + | wantErr error | |||
| 230 | + | check func(t *testing.T, err error) | |||
| 231 | + | }{ | |||
| 232 | + | { | |||
| 233 | + | name: "oversized stdout", | |||
| 234 | + | command: func(ctx context.Context) *exec.Cmd { | |||
| 235 | + | return exec.CommandContext(ctx, "sh", "-c", | |||
| 236 | + | fmt.Sprintf("head -c %d /dev/zero", maxCatalogBytes+1)) | |||
| 237 | + | }, | |||
| 238 | + | wantErr: errCatalogTooLarge, | |||
| 239 | + | check: func(t *testing.T, err error) { | |||
| 240 | + | t.Helper() | |||
| 241 | + | if strings.Contains(err.Error(), strings.Repeat("\x00", 64)) { | |||
| 242 | + | t.Fatalf("error embeds oversized stdout: %v", err) | |||
| 243 | + | } | |||
| 244 | + | }, | |||
| 245 | + | }, | |||
| 246 | + | { | |||
| 247 | + | name: "oversized stderr", | |||
| 248 | + | command: func(ctx context.Context) *exec.Cmd { | |||
| 249 | + | return exec.CommandContext(ctx, "sh", "-c", | |||
| 250 | + | fmt.Sprintf("head -c %d /dev/zero >&2; exit 1", maxCatalogDiagnosticBytes+1)) | |||
| 251 | + | }, | |||
| 252 | + | wantErr: errCatalogTooLarge, | |||
| 253 | + | check: func(t *testing.T, err error) { | |||
| 254 | + | t.Helper() | |||
| 255 | + | if strings.Contains(err.Error(), strings.Repeat("\x00", 64)) { | |||
| 256 | + | t.Fatalf("error embeds oversized stderr: %v", err) | |||
| 257 | + | } | |||
| 258 | + | if errors.Is(err, errCatalogSSH) { | |||
| 259 | + | t.Fatalf("oversized stderr must not wrap SSH with captured output: %v", err) | |||
| 260 | + | } | |||
| 261 | + | }, | |||
| 262 | + | }, | |||
| 263 | + | { | |||
| 264 | + | name: "ansi control stderr", | |||
| 265 | + | command: func(ctx context.Context) *exec.Cmd { | |||
| 266 | + | cmd := exec.CommandContext(ctx, "sh", "-c", "cat >&2; exit 255") | |||
| 267 | + | cmd.Stdin = strings.NewReader("\x1b[31mconnection refused\x1b[0m\x07") | |||
| 268 | + | return cmd | |||
| 269 | + | }, | |||
| 270 | + | wantErr: errCatalogSSH, | |||
| 271 | + | check: func(t *testing.T, err error) { | |||
| 272 | + | t.Helper() | |||
| 273 | + | msg := err.Error() | |||
| 274 | + | if strings.ContainsRune(msg, '\x1b') || strings.ContainsRune(msg, '\x07') { | |||
| 275 | + | t.Fatalf("error retains control/ANSI bytes: %q", msg) | |||
| 276 | + | } | |||
| 277 | + | if strings.Contains(msg, "[31m") || strings.Contains(msg, "[0m") { | |||
| 278 | + | t.Fatalf("error retains terminal escape remnants: %q", msg) | |||
| 279 | + | } | |||
| 280 | + | if !strings.Contains(msg, "connection refused") { | |||
| 281 | + | t.Fatalf("error = %q, want printable diagnostic text preserved", msg) | |||
| 282 | + | } | |||
| 283 | + | }, | |||
| 284 | + | }, | |||
| 285 | + | } | |||
| 286 | + | ||||
| 287 | + | for _, tt := range tests { | |||
| 288 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 289 | + | t.Parallel() | |||
| 290 | + | client := &CatalogClient{ | |||
| 291 | + | command: func(ctx context.Context, name string, args ...string) *exec.Cmd { | |||
| 292 | + | return tt.command(ctx) | |||
| 293 | + | }, | |||
| 294 | + | } | |||
| 295 | + | _, err := client.List(context.Background(), "arch") | |||
| 296 | + | if !errors.Is(err, tt.wantErr) { | |||
| 297 | + | t.Fatalf("List() error = %v, want %v", err, tt.wantErr) | |||
| 298 | + | } | |||
| 299 | + | if tt.check != nil { | |||
| 300 | + | tt.check(t, err) | |||
| 301 | + | } | |||
| 302 | + | }) | |||
| 303 | + | } | |||
| 304 | + | } | |||
| 305 | + | ||||
| 306 | + | func TestBoundedBufferLimits(t *testing.T) { | |||
| 307 | + | t.Parallel() | |||
| 308 | + | ||||
| 309 | + | tests := []struct { | |||
| 310 | + | name string | |||
| 311 | + | limit int | |||
| 312 | + | chunks [][]byte | |||
| 313 | + | wantLen int | |||
| 314 | + | wantOver bool | |||
| 315 | + | wantBytes string | |||
| 316 | + | }{ | |||
| 317 | + | { | |||
| 318 | + | name: "under limit", | |||
| 319 | + | limit: 8, | |||
| 320 | + | chunks: [][]byte{[]byte("abcd"), []byte("ef")}, | |||
| 321 | + | wantLen: 6, | |||
| 322 | + | wantOver: false, | |||
| 323 | + | wantBytes: "abcdef", | |||
| 324 | + | }, | |||
| 325 | + | { | |||
| 326 | + | name: "exact limit", | |||
| 327 | + | limit: 4, | |||
| 328 | + | chunks: [][]byte{[]byte("abcd")}, | |||
| 329 | + | wantLen: 4, | |||
| 330 | + | wantOver: false, | |||
| 331 | + | wantBytes: "abcd", | |||
| 332 | + | }, | |||
| 333 | + | { | |||
| 334 | + | name: "overflow discards remainder", | |||
| 335 | + | limit: 4, | |||
| 336 | + | chunks: [][]byte{[]byte("abc"), []byte("defgh")}, | |||
| 337 | + | wantLen: 4, | |||
| 338 | + | wantOver: true, | |||
| 339 | + | wantBytes: "abcd", | |||
| 340 | + | }, | |||
| 341 | + | { | |||
| 342 | + | name: "further writes stay discarded", | |||
| 343 | + | limit: 2, | |||
| 344 | + | chunks: [][]byte{[]byte("abcdef"), []byte("xyz")}, | |||
| 345 | + | wantLen: 2, | |||
| 346 | + | wantOver: true, | |||
| 347 | + | wantBytes: "ab", | |||
| 348 | + | }, | |||
| 349 | + | } | |||
| 350 | + | ||||
| 351 | + | for _, tt := range tests { | |||
| 352 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 353 | + | t.Parallel() | |||
| 354 | + | var buf boundedBuffer | |||
| 355 | + | buf.limit = tt.limit | |||
| 356 | + | for _, chunk := range tt.chunks { | |||
| 357 | + | n, err := buf.Write(chunk) | |||
| 358 | + | if err != nil { | |||
| 359 | + | t.Fatalf("Write(%q) error = %v", chunk, err) | |||
| 360 | + | } | |||
| 361 | + | if n != len(chunk) { | |||
| 362 | + | t.Fatalf("Write(%q) n = %d, want %d", chunk, n, len(chunk)) | |||
| 363 | + | } | |||
| 364 | + | } | |||
| 365 | + | if buf.overflow != tt.wantOver { | |||
| 366 | + | t.Fatalf("overflow = %v, want %v", buf.overflow, tt.wantOver) | |||
| 367 | + | } | |||
| 368 | + | if got := len(buf.Bytes()); got != tt.wantLen { | |||
| 369 | + | t.Fatalf("len = %d, want %d", got, tt.wantLen) | |||
| 370 | + | } | |||
| 371 | + | if got := string(buf.Bytes()); got != tt.wantBytes { | |||
| 372 | + | t.Fatalf("bytes = %q, want %q", got, tt.wantBytes) | |||
| 373 | + | } | |||
| 374 | + | }) | |||
| 375 | + | } | |||
| 376 | + | } | |||
| 377 | + | ||||
| 223 | func stdoutCmd(ctx context.Context, stdout string) *exec.Cmd { | 378 | func stdoutCmd(ctx context.Context, stdout string) *exec.Cmd { | ||
| 224 | cmd := exec.CommandContext(ctx, "cat") | 379 | cmd := exec.CommandContext(ctx, "cat") | ||
| 225 | cmd.Stdin = strings.NewReader(stdout) | 380 | cmd.Stdin = strings.NewReader(stdout) | ||