From 4f8f30dac32fb150f8a29f24d2124b0cc1d2ff8e Mon Sep 17 00:00:00 2001 From: Ziemek Borowski Date: Sun, 14 Jun 2026 09:49:15 +0200 Subject: [PATCH 1/5] cr 1. Fix POP3 ReadResponseWithTimeout (item 2.2) --- ...ODE_REVIEW_AND_IMPROVEMENTS_2026_06_13.md} | 0 ...CODE_REVIEW_AND_IMPROVEMENTS_2026_06_14.md | 94 ++++++ internal/pop3/protocol/responses_test.go | 117 ++++++++ internal/protocols/ews/autodiscover.go | 3 +- internal/protocols/imap/listfolders.go | 3 +- internal/protocols/imap/testauth.go | 11 +- internal/protocols/imap/utils.go | 10 - internal/protocols/jmap/testauth.go | 9 +- internal/protocols/jmap/utils.go | 41 --- internal/protocols/jmap/utils_test.go | 69 ----- internal/protocols/msgraph/auth.go | 3 +- internal/protocols/msgraph/utils.go | 17 -- internal/protocols/msgraph/utils_test.go | 48 ---- internal/protocols/pop3/listmail.go | 3 +- internal/protocols/pop3/testauth.go | 13 +- internal/protocols/pop3/utils.go | 10 - internal/protocols/smtp/sendmail.go | 7 +- internal/protocols/smtp/testauth.go | 9 +- internal/protocols/smtp/utils.go | 51 ---- internal/protocols/smtp/utils_test.go | 269 ------------------ 20 files changed, 246 insertions(+), 541 deletions(-) rename cr-results/{CODE_REVIEW_AND_IMPROVEMENTS_2026_06-13.md => CODE_REVIEW_AND_IMPROVEMENTS_2026_06_13.md} (100%) create mode 100644 cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06_14.md create mode 100644 internal/pop3/protocol/responses_test.go delete mode 100644 internal/protocols/imap/utils.go delete mode 100644 internal/protocols/jmap/utils.go delete mode 100644 internal/protocols/jmap/utils_test.go delete mode 100644 internal/protocols/pop3/utils.go delete mode 100644 internal/protocols/smtp/utils.go delete mode 100644 internal/protocols/smtp/utils_test.go diff --git a/cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06-13.md b/cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06_13.md similarity index 100% rename from cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06-13.md rename to cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06_13.md diff --git a/cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06_14.md b/cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06_14.md new file mode 100644 index 0000000..780ecf2 --- /dev/null +++ b/cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06_14.md @@ -0,0 +1,94 @@ +# Code Review Results - gomailtesttool + +**Scope:** Full codebase review (122 Go files, ~25k lines), 2026-06-14. +`go build ./...` and `go vet ./...` both pass cleanly. + +## 0. Status of Previous Findings + +All issues from the prior review in this file have been fixed and are +tracked as completed in `TODO.md`: + +- Broken `MaskAccessToken`/`MaskPassword` in `internal/common/security/masking.go` + (previously exposed full short tokens/passwords) — fixed. +- API-key middleware bypass on empty configured key + (`internal/serve/server.go`) — now fails closed. +- Default bind address changed from `0.0.0.0` to `127.0.0.1` + (`internal/serve/cmd.go`). +- SMTP response-reader goroutine leak in + `internal/smtp/protocol/responses.go` — now uses `conn.SetReadDeadline` + instead of a goroutine + `time.After`. +- Missing HTTP server `IdleTimeout` (`internal/serve/server.go`) — now set + to 60s. + +Notably, TODO.md records that fixing the masking bugs required applying +"the same fix" separately to duplicate copies of the masking functions in +`internal/protocols/jmap/utils.go` and `internal/protocols/smtp/utils.go`. +That duplication is itself the main new finding below (1.1). + +## 1. Security Issues (in context of a credential-handling testing tool) + +### 1.1 Major: Credential-masking logic is duplicated across 6 packages, creating drift risk + +- **Location:** + - `internal/common/security/masking.go` (canonical, fixed version) + - `internal/protocols/smtp/utils.go` (`maskPassword`, `maskUsername`, `maskAccessToken`) + - `internal/protocols/jmap/utils.go` (`maskUsername`, `maskPassword`, `maskAccessToken`) + - `internal/protocols/imap/utils.go` (`maskUsername`) + - `internal/protocols/pop3/utils.go` (`maskUsername`) + - `internal/protocols/msgraph/utils.go` (`maskSecret`, `maskGUID`) +- **Issue:** Six packages each carry their own private copies of what is + essentially the same masking logic, with subtly different thresholds and + output formats (e.g. `smtp.maskAccessToken` masks tokens <= 16 chars + completely, while `jmap.maskAccessToken` and `common/security.MaskAccessToken` + show 2 chars on each side for 9-16 char tokens). Only + `internal/devtools/env/env.go` actually imports `common/security`. +- **Impact:** When the masking bugs were fixed in `common/security`, the + fixes had to be re-applied by hand to the `jmap` and `smtp` copies (per + TODO.md). Any future fix to masking behavior is likely to miss one or + more of the 5 duplicate copies, silently leaving a weaker/incorrect + masking implementation in some protocol packages. +- **Recommendation:** Have all protocol packages call + `internal/common/security` directly and delete the local `maskX` + helpers. This removes ~5 duplicate implementations and ensures future + fixes apply everywhere at once. + +### 1.2 Minor: `internal/protocols/msgraph/utils.go:maskSecret` has a masking-weakness bug (currently dead code) + +- **Location:** `internal/protocols/msgraph/utils.go:41-47` +- **Issue:** For secrets of 9-16 characters, `secret[:4] + "********" + secret[len(secret)-4:]` exposes 8 of the secret's characters (e.g. a 9-char secret exposes 8/9 chars; the two 4-char windows overlap). This is the same class of bug already fixed in `common/security.MaskSecret` (which now returns `secret[:4] + "****"` for secrets > 4 chars, with no trailing reveal). +- **Impact:** Currently `maskSecret` is unused (only `maskGUID` is called, in `auth.go:64`), so there's no live exposure. But it's a trap for the next person who wires it up to log a client secret. +- **Recommendation:** Remove this dead function as part of consolidating onto `common/security` (see 1.1), or fix it to match the safer canonical behavior if it must stay. + +### 1.3 Low: `internal/pop3/protocol/responses.go:ReadResponseWithTimeout` silently ignores its `timeout` parameter + +- **Location:** `internal/pop3/protocol/responses.go:43-47` + ```go + func ReadResponseWithTimeout(reader *bufio.Reader, timeout time.Duration) (*POP3Response, error) { + // For now, just use the basic read - timeout should be set on the connection + return ReadResponse(reader) + } + ``` +- **Issue:** The function accepts a `timeout` but never uses it — it's pure dead code with a misleading signature/doc comment ("reads a response with a timeout"). It is not called from anywhere in the codebase. +- **Impact:** Low today since it's unused, but if a future caller adopts it expecting `SetReadDeadline`-style enforcement (as SMTP's `ReadResponseWithTimeout` now correctly does), they'll get an unbounded read and a potential hang against a misbehaving POP3 server. +- **Related gap:** The live POP3 command path (`internal/protocols/pop3/pop3_client.go`, calls to `protocol.ReadResponse` at lines 101, 139, 240, 253, 280, 303) sets no read deadline at all for normal commands — only the `QUIT`/close path (line 361) sets a 5s deadline. SMTP, by contrast, wraps every response read in `ReadResponseWithTimeout(conn, ...)` which sets and clears a deadline. Consider giving POP3 the same per-command deadline treatment SMTP has, rather than keeping the unused/broken `ReadResponseWithTimeout` helper. + +## 2. Code Quality / Correctness + +### 2.1 Minor: EWS Autodiscover prints a hardcoded `https://` scheme that can mismatch the actual request URL + +- **Location:** `internal/protocols/ews/autodiscover.go:53` + ```go + fmt.Printf(" Endpoint: https://%s:%d%s\n\n", config.Host, config.Port, config.AutodiscoverPath) + ``` +- **Issue:** `NewEWSClient` (`internal/protocols/ews/ews_client.go:118-121`) computes `scheme := "https"` but switches to `"http"` when `config.Port == 80`, and builds `autodiscoverURL` from that scheme. The printed message in `autodiscover.go` always says `https://`, regardless of the actual scheme used for the request. +- **Impact:** Cosmetic — diagnostic output can show the wrong scheme when testing against port 80, which is confusing when comparing the printed endpoint to packet captures or proxy logs. +- **Recommendation:** Use `ewsClient.AutodiscoverUrl()` (already exposed) for the printed endpoint instead of re-deriving it with a hardcoded scheme. + +## 3. Summary + +The codebase is in good shape overall — `go vet` is clean, and every issue +from the previous review has been addressed. The main opportunity for this +pass is **1.1**: consolidating the five duplicate masking-helper copies onto +`internal/common/security` would remove the structural cause of the +previous masking bugs recurring, and incidentally cleans up the dead/buggy +code in **1.2** and **1.3**. diff --git a/internal/pop3/protocol/responses_test.go b/internal/pop3/protocol/responses_test.go new file mode 100644 index 0000000..382c376 --- /dev/null +++ b/internal/pop3/protocol/responses_test.go @@ -0,0 +1,117 @@ +package protocol + +import ( + "bufio" + "net" + "strings" + "testing" + "time" +) + +func newPipeConn(t *testing.T) (net.Conn, *bufio.Reader, net.Conn) { + t.Helper() + client, server := net.Pipe() + t.Cleanup(func() { + client.Close() + server.Close() + }) + return client, bufio.NewReader(client), server +} + +func TestReadResponseWithTimeout_Success(t *testing.T) { + tests := []struct { + name string + input string + timeout time.Duration + wantSuccess bool + wantMessage string + }{ + { + name: "Valid +OK response within timeout", + input: "+OK done\r\n", + timeout: 1 * time.Second, + wantSuccess: true, + wantMessage: "done", + }, + { + name: "Valid -ERR response within timeout", + input: "-ERR no such mailbox\r\n", + timeout: 1 * time.Second, + wantSuccess: false, + wantMessage: "no such mailbox", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, reader, server := newPipeConn(t) + go func() { + _, _ = server.Write([]byte(tt.input)) + }() + + resp, err := ReadResponseWithTimeout(client, reader, tt.timeout) + if err != nil { + t.Fatalf("ReadResponseWithTimeout() unexpected error: %v", err) + } + + if resp.Success != tt.wantSuccess { + t.Errorf("ReadResponseWithTimeout() Success = %v, want %v", resp.Success, tt.wantSuccess) + } + + if resp.Message != tt.wantMessage { + t.Errorf("ReadResponseWithTimeout() Message = %q, want %q", resp.Message, tt.wantMessage) + } + }) + } +} + +func TestReadResponseWithTimeout_Timeout(t *testing.T) { + // Server side never writes, simulating a hanging server. + client, reader, _ := newPipeConn(t) + + timeout := 50 * time.Millisecond + + start := time.Now() + resp, err := ReadResponseWithTimeout(client, reader, timeout) + elapsed := time.Since(start) + + if err == nil { + t.Fatal("ReadResponseWithTimeout() expected timeout error, got nil") + } + + if resp != nil { + t.Errorf("ReadResponseWithTimeout() expected nil response on timeout, got %v", resp) + } + + if !strings.Contains(err.Error(), "timeout") { + t.Errorf("ReadResponseWithTimeout() error should contain 'timeout', got: %v", err) + } + + if elapsed < timeout || elapsed > timeout+200*time.Millisecond { + t.Errorf("ReadResponseWithTimeout() elapsed time = %v, want approximately %v", elapsed, timeout) + } +} + +func TestReadResponseWithTimeout_DeadlineCleared(t *testing.T) { + client, reader, server := newPipeConn(t) + + // First read times out. + if _, err := ReadResponseWithTimeout(client, reader, 50*time.Millisecond); err == nil { + t.Fatal("expected timeout error on first read") + } + + // A subsequent read without a deadline should succeed once data arrives, + // proving the earlier deadline was cleared. + go func() { + _, _ = server.Write([]byte("+OK ready\r\n")) + }() + + resp, err := ReadResponseWithTimeout(client, reader, 1*time.Second) + if err != nil { + t.Fatalf("ReadResponseWithTimeout() unexpected error after deadline clear: %v", err) + } + + if !resp.Success || resp.Message != "ready" { + t.Errorf("ReadResponseWithTimeout() = %+v, want Success=true Message=%q", resp, "ready") + } +} diff --git a/internal/protocols/ews/autodiscover.go b/internal/protocols/ews/autodiscover.go index 2b4b032..55346a0 100644 --- a/internal/protocols/ews/autodiscover.go +++ b/internal/protocols/ews/autodiscover.go @@ -50,7 +50,6 @@ type userSetting struct { // autodiscover sends a SOAP GetUserSettings request to the Autodiscover endpoint. func autodiscover(ctx context.Context, config *Config, csvLogger logger.Logger, slogLogger *slog.Logger) error { fmt.Printf("Testing EWS Autodiscover for %s\n", config.Username) - fmt.Printf(" Endpoint: https://%s:%d%s\n\n", config.Host, config.Port, config.AutodiscoverPath) if shouldWrite, _ := csvLogger.ShouldWriteHeader(); shouldWrite { if err := csvLogger.WriteHeader([]string{ @@ -68,6 +67,8 @@ func autodiscover(ctx context.Context, config *Config, csvLogger logger.Logger, return err } + fmt.Printf(" Endpoint: %s\n\n", ewsClient.AutodiscoverUrl()) + logger.LogDebug(slogLogger, "Sending Autodiscover GetUserSettings", "email", config.Username) start := time.Now() diff --git a/internal/protocols/imap/listfolders.go b/internal/protocols/imap/listfolders.go index 90d9439..68f827a 100644 --- a/internal/protocols/imap/listfolders.go +++ b/internal/protocols/imap/listfolders.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/ziembor/gomailtesttool/internal/common/logger" + "github.com/ziembor/gomailtesttool/internal/common/security" ) // listFolders lists all mailbox folders. @@ -74,7 +75,7 @@ func listFolders(ctx context.Context, config *Config, csvLogger logger.Logger, s if authErr != nil { logger.LogError(slogLogger, "Authentication failed", "error", authErr, - "username", maskUsername(config.Username)) + "username", security.MaskUsername(config.Username)) if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), diff --git a/internal/protocols/imap/testauth.go b/internal/protocols/imap/testauth.go index a3dc01f..4e267bf 100644 --- a/internal/protocols/imap/testauth.go +++ b/internal/protocols/imap/testauth.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/ziembor/gomailtesttool/internal/common/logger" + "github.com/ziembor/gomailtesttool/internal/common/security" ) // testAuth tests IMAP authentication. @@ -32,7 +33,7 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), "", "", "FAILURE", err.Error(), + security.MaskUsername(config.Username), "", "", "FAILURE", err.Error(), }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } @@ -86,12 +87,12 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog if authErr != nil { logger.LogError(slogLogger, "Authentication failed", "error", authErr, - "username", maskUsername(config.Username), + "username", security.MaskUsername(config.Username), "method", authMethod) if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), authMechanisms, authMethod, "FAILURE", authErr.Error(), + security.MaskUsername(config.Username), authMechanisms, authMethod, "FAILURE", authErr.Error(), }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } @@ -99,12 +100,12 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog } logger.LogInfo(slogLogger, "Authentication successful", - "username", maskUsername(config.Username), + "username", security.MaskUsername(config.Username), "method", authMethod) if logErr := csvLogger.WriteRow([]string{ config.Action, "SUCCESS", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), authMechanisms, authMethod, "SUCCESS", "", + security.MaskUsername(config.Username), authMechanisms, authMethod, "SUCCESS", "", }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } diff --git a/internal/protocols/imap/utils.go b/internal/protocols/imap/utils.go deleted file mode 100644 index fc6a98d..0000000 --- a/internal/protocols/imap/utils.go +++ /dev/null @@ -1,10 +0,0 @@ -package imap - -// maskUsername masks a username for safe logging. -// Shows first 2 and last 2 characters with **** in between. -func maskUsername(username string) string { - if len(username) <= 4 { - return "****" - } - return username[:2] + "****" + username[len(username)-2:] -} diff --git a/internal/protocols/jmap/testauth.go b/internal/protocols/jmap/testauth.go index f3282e3..54a5e46 100644 --- a/internal/protocols/jmap/testauth.go +++ b/internal/protocols/jmap/testauth.go @@ -6,6 +6,7 @@ import ( "log/slog" "github.com/ziembor/gomailtesttool/internal/common/logger" + "github.com/ziembor/gomailtesttool/internal/common/security" "github.com/ziembor/gomailtesttool/internal/jmap/protocol" ) @@ -35,12 +36,12 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog logger.LogError(slogLogger, "JMAP authentication failed", "error", err, "host", config.Host, - "username", maskUsername(config.Username), + "username", security.MaskUsername(config.Username), "auth_method", authMethod) if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), authMethod, "", "", err.Error(), + security.MaskUsername(config.Username), authMethod, "", "", err.Error(), }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } @@ -88,7 +89,7 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog // Log success to CSV if logErr := csvLogger.WriteRow([]string{ config.Action, "SUCCESS", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), authMethod, session.APIURL, + security.MaskUsername(config.Username), authMethod, session.APIURL, fmt.Sprintf("%d", session.GetAccountCount()), "", }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) @@ -96,7 +97,7 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog logger.LogInfo(slogLogger, "JMAP authentication test completed", "host", config.Host, - "username", maskUsername(config.Username), + "username", security.MaskUsername(config.Username), "auth_method", authMethod, "accounts", session.GetAccountCount(), "has_mail", session.HasMailCapability()) diff --git a/internal/protocols/jmap/utils.go b/internal/protocols/jmap/utils.go deleted file mode 100644 index 90bd151..0000000 --- a/internal/protocols/jmap/utils.go +++ /dev/null @@ -1,41 +0,0 @@ -package jmap - -// maskUsername masks a username for safe logging. -// Shows first 2 and last 2 characters with **** in between. -func maskUsername(username string) string { - if len(username) <= 4 { - return "****" - } - return username[:2] + "****" + username[len(username)-2:] -} - -// maskPassword masks a password for safe logging. -// Shows first 2 and last 2 characters with **** in between. -// Passwords of 8 characters or fewer are fully masked, since revealing 4 -// characters of a password that short would disclose most or all of it. -func maskPassword(password string) string { - if len(password) == 0 { - return "" - } - if len(password) <= 8 { - return "****" - } - return password[:2] + "****" + password[len(password)-2:] -} - -// maskAccessToken masks an access token for safe logging. -// Shows first 8 and last 4 characters with ... in between for tokens longer -// than 16 characters. Tokens of 9-16 characters show only first 2 and last 2. -// Tokens of 8 characters or fewer are fully masked. -func maskAccessToken(token string) string { - if len(token) == 0 { - return "" - } - if len(token) <= 8 { - return "****" - } - if len(token) <= 16 { - return token[:2] + "****" + token[len(token)-2:] - } - return token[:8] + "..." + token[len(token)-4:] -} diff --git a/internal/protocols/jmap/utils_test.go b/internal/protocols/jmap/utils_test.go deleted file mode 100644 index 53afc4f..0000000 --- a/internal/protocols/jmap/utils_test.go +++ /dev/null @@ -1,69 +0,0 @@ -package jmap - -import ( - "testing" -) - -func TestMaskUsername(t *testing.T) { - tests := []struct { - input string - expected string - }{ - {"user@example.com", "us****om"}, - {"ab@example.com", "ab****om"}, - {"test", "****"}, - {"ab", "****"}, - {"a", "****"}, - } - - for _, tt := range tests { - result := maskUsername(tt.input) - if result != tt.expected { - t.Errorf("maskUsername(%q) = %q, want %q", tt.input, result, tt.expected) - } - } -} - -func TestMaskPassword(t *testing.T) { - tests := []struct { - input string - expected string - }{ - {"secretpassword", "se****rd"}, - {"123456789", "12****89"}, - {"password", "****"}, - {"test", "****"}, - {"ab", "****"}, - {"", ""}, - } - - for _, tt := range tests { - result := maskPassword(tt.input) - if result != tt.expected { - t.Errorf("maskPassword(%q) = %q, want %q", tt.input, result, tt.expected) - } - } -} - -func TestMaskAccessToken(t *testing.T) { - tests := []struct { - input string - expected string - }{ - {"ya29.a0ARrdaM_1234567890abcdefghij", "ya29.a0A...ghij"}, - {"1234567890123456", "12****56"}, - {"short", "****"}, - {"1234", "****"}, - {"abc", "****"}, - {"ab", "****"}, - {"a", "****"}, - {"", ""}, - } - - for _, tt := range tests { - result := maskAccessToken(tt.input) - if result != tt.expected { - t.Errorf("maskAccessToken(%q) = %q, want %q", tt.input, result, tt.expected) - } - } -} diff --git a/internal/protocols/msgraph/auth.go b/internal/protocols/msgraph/auth.go index f09fa1a..310c3e5 100644 --- a/internal/protocols/msgraph/auth.go +++ b/internal/protocols/msgraph/auth.go @@ -15,6 +15,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/golang-jwt/jwt/v5" msgraphsdk "github.com/microsoftgraph/msgraph-sdk-go" + "github.com/ziembor/gomailtesttool/internal/common/security" "software.sslmate.com/src/go-pkcs12" ) @@ -61,7 +62,7 @@ func (c *BearerTokenCredential) GetToken(ctx context.Context, options policy.Tok // NewGraphServiceClient creates credentials and initializes the Microsoft Graph SDK client. func NewGraphServiceClient(ctx context.Context, config *Config, logger *slog.Logger) (*msgraphsdk.GraphServiceClient, error) { // Setup Authentication - logDebug(logger, "Setting up Microsoft Graph client", "tenantID", maskGUID(config.TenantID), "clientID", maskGUID(config.ClientID)) + logDebug(logger, "Setting up Microsoft Graph client", "tenantID", security.MaskGUID(config.TenantID), "clientID", security.MaskGUID(config.ClientID)) cred, err := getCredential(config.TenantID, config.ClientID, config.Secret, config.PfxPath, config.PfxPass, config.Thumbprint, config, logger) if err != nil { diff --git a/internal/protocols/msgraph/utils.go b/internal/protocols/msgraph/utils.go index a84ea42..ba8cd4c 100644 --- a/internal/protocols/msgraph/utils.go +++ b/internal/protocols/msgraph/utils.go @@ -37,23 +37,6 @@ func logVerbose(verbose bool, format string, args ...interface{}) { } } -// maskSecret masks a secret for display, showing only first and last 4 characters -func maskSecret(secret string) string { - if len(secret) <= 8 { - return "********" - } - // Show first 4 and last 4 characters - return secret[:4] + "********" + secret[len(secret)-4:] -} - -// maskGUID masks a GUID for logging, showing only first 4 and last 4 characters -func maskGUID(guid string) string { - if len(guid) <= 8 { - return "****" - } - return guid[:4] + "****-****-****-****" + guid[len(guid)-4:] -} - // ifEmpty returns defaultVal if s is empty, otherwise returns s func ifEmpty(s, defaultVal string) string { if s == "" { diff --git a/internal/protocols/msgraph/utils_test.go b/internal/protocols/msgraph/utils_test.go index 07c7e69..160b27f 100644 --- a/internal/protocols/msgraph/utils_test.go +++ b/internal/protocols/msgraph/utils_test.go @@ -111,54 +111,6 @@ func TestValidateMessageID(t *testing.T) { } } -// TestMaskSecret tests secret masking for logging (security helper) -func TestMaskSecret(t *testing.T) { - tests := []struct { - name string - secret string - expected string - }{ - {"Short secret (<= 8 chars)", "secret", "********"}, - {"Short secret (8 chars)", "12345678", "********"}, - {"Normal secret (12 chars)", "secret123456", "secr********3456"}, - {"Long secret", "this_is_a_very_long_secret_key", "this********_key"}, - {"Empty secret", "", "********"}, - {"Minimum maskable (9 chars)", "123456789", "1234********6789"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := maskSecret(tt.secret) - if result != tt.expected { - t.Errorf("maskSecret(%q) = %q, want %q", tt.secret, result, tt.expected) - } - }) - } -} - -// TestMaskGUID tests GUID masking for logging (security helper) -func TestMaskGUID(t *testing.T) { - tests := []struct { - name string - guid string - expected string - }{ - {"Standard GUID", "12345678-1234-1234-1234-123456789012", "1234****-****-****-****9012"}, - {"Short GUID (<= 8 chars)", "1234", "****"}, - {"Empty GUID", "", "****"}, - {"Minimum maskable (9 chars)", "123456789", "1234****-****-****-****6789"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := maskGUID(tt.guid) - if result != tt.expected { - t.Errorf("maskGUID(%q) = %q, want %q", tt.guid, result, tt.expected) - } - }) - } -} - // TestIfEmpty tests the conditional string helper func TestIfEmpty(t *testing.T) { tests := []struct { diff --git a/internal/protocols/pop3/listmail.go b/internal/protocols/pop3/listmail.go index 6ccb91a..e3fd70b 100644 --- a/internal/protocols/pop3/listmail.go +++ b/internal/protocols/pop3/listmail.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/ziembor/gomailtesttool/internal/common/logger" + "github.com/ziembor/gomailtesttool/internal/common/security" ) // listMail lists messages in the mailbox. @@ -88,7 +89,7 @@ func listMail(ctx context.Context, config *Config, csvLogger logger.Logger, slog if authErr != nil { logger.LogError(slogLogger, "Authentication failed", "error", authErr, - "username", maskUsername(config.Username)) + "username", security.MaskUsername(config.Username)) if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), diff --git a/internal/protocols/pop3/testauth.go b/internal/protocols/pop3/testauth.go index ca89838..ef6a193 100644 --- a/internal/protocols/pop3/testauth.go +++ b/internal/protocols/pop3/testauth.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/ziembor/gomailtesttool/internal/common/logger" + "github.com/ziembor/gomailtesttool/internal/common/security" ) // testAuth tests POP3 authentication. @@ -32,7 +33,7 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), "", "FAILURE", err.Error(), + security.MaskUsername(config.Username), "", "FAILURE", err.Error(), }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } @@ -50,7 +51,7 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), "", "FAILURE", fmt.Sprintf("STLS failed: %v", err), + security.MaskUsername(config.Username), "", "FAILURE", fmt.Sprintf("STLS failed: %v", err), }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } @@ -90,12 +91,12 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog if authErr != nil { logger.LogError(slogLogger, "Authentication failed", "error", authErr, - "username", maskUsername(config.Username), + "username", security.MaskUsername(config.Username), "method", authMethod) if logErr := csvLogger.WriteRow([]string{ config.Action, "FAILURE", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), authMethod, "FAILURE", authErr.Error(), + security.MaskUsername(config.Username), authMethod, "FAILURE", authErr.Error(), }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } @@ -103,12 +104,12 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog } logger.LogInfo(slogLogger, "Authentication successful", - "username", maskUsername(config.Username), + "username", security.MaskUsername(config.Username), "method", authMethod) if logErr := csvLogger.WriteRow([]string{ config.Action, "SUCCESS", config.Host, fmt.Sprintf("%d", config.Port), - maskUsername(config.Username), authMethod, "SUCCESS", "", + security.MaskUsername(config.Username), authMethod, "SUCCESS", "", }); logErr != nil { logger.LogError(slogLogger, "Failed to write CSV row", "error", logErr) } diff --git a/internal/protocols/pop3/utils.go b/internal/protocols/pop3/utils.go deleted file mode 100644 index ade3cdb..0000000 --- a/internal/protocols/pop3/utils.go +++ /dev/null @@ -1,10 +0,0 @@ -package pop3 - -// maskUsername masks a username for safe logging. -// Shows first 2 and last 2 characters with **** in between. -func maskUsername(username string) string { - if len(username) <= 4 { - return "****" - } - return username[:2] + "****" + username[len(username)-2:] -} diff --git a/internal/protocols/smtp/sendmail.go b/internal/protocols/smtp/sendmail.go index 7930820..c06c8b4 100644 --- a/internal/protocols/smtp/sendmail.go +++ b/internal/protocols/smtp/sendmail.go @@ -15,6 +15,7 @@ import ( "github.com/ziembor/gomailtesttool/internal/common/email" "github.com/ziembor/gomailtesttool/internal/common/logger" + "github.com/ziembor/gomailtesttool/internal/common/security" smtptls "github.com/ziembor/gomailtesttool/internal/smtp/tls" ) @@ -189,9 +190,9 @@ func SendMail(ctx context.Context, config *Config, csvLogger logger.Logger, slog if err := client.Auth(config.Username, config.Password, config.AccessToken, []string{methodToUse}); err != nil { logger.LogError(slogLogger, "Authentication failed", "error", err, - "username", maskUsername(config.Username), - "password", maskPassword(config.Password), - "accesstoken", maskAccessToken(config.AccessToken), + "username", security.MaskUsername(config.Username), + "password", security.MaskPassword(config.Password), + "accesstoken", security.MaskAccessToken(config.AccessToken), "method", methodToUse) // Show TLS cipher information on auth failure if verbose and TLS was used diff --git a/internal/protocols/smtp/testauth.go b/internal/protocols/smtp/testauth.go index e534f6b..65b4a5e 100644 --- a/internal/protocols/smtp/testauth.go +++ b/internal/protocols/smtp/testauth.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/ziembor/gomailtesttool/internal/common/logger" + "github.com/ziembor/gomailtesttool/internal/common/security" smtptls "github.com/ziembor/gomailtesttool/internal/smtp/tls" ) @@ -185,7 +186,7 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog } fmt.Printf("Attempting authentication with method: %s\n", methodUsed) - logger.LogDebug(slogLogger, "Authenticating", "method", methodUsed, "username", maskUsername(config.Username)) + logger.LogDebug(slogLogger, "Authenticating", "method", methodUsed, "username", security.MaskUsername(config.Username)) // Authenticate err = client.Auth(config.Username, config.Password, config.AccessToken, []string{methodUsed}) @@ -201,9 +202,9 @@ func testAuth(ctx context.Context, config *Config, csvLogger logger.Logger, slog fmt.Printf("\n✗ Authentication failed: %v\n", err) logger.LogError(slogLogger, "Authentication failed", "error", err, - "username", maskUsername(config.Username), - "password", maskPassword(config.Password), - "accesstoken", maskAccessToken(config.AccessToken), + "username", security.MaskUsername(config.Username), + "password", security.MaskPassword(config.Password), + "accesstoken", security.MaskAccessToken(config.AccessToken), "method", methodUsed) // Show TLS cipher information on auth failure if verbose and TLS was used diff --git a/internal/protocols/smtp/utils.go b/internal/protocols/smtp/utils.go deleted file mode 100644 index b89837a..0000000 --- a/internal/protocols/smtp/utils.go +++ /dev/null @@ -1,51 +0,0 @@ -package smtp - -// maskPassword masks a password for display in logs and error messages. -// For passwords <= 8 characters, returns "****" (revealing 4 characters of a -// password that short would disclose most or all of it) -// For longer passwords, shows first 2 and last 2 characters with **** in between -// This prevents password exposure in logs while allowing identification of which credential was used. -// -// Examples: -// - "abc" -> "****" -// - "password123" -> "pa****23" -// - "MySecretP@ss" -> "My****ss" -func maskPassword(password string) string { - if len(password) <= 8 { - return "****" - } - // Show first 2 and last 2 characters - return password[:2] + "****" + password[len(password)-2:] -} - -// maskUsername masks a username for display in logs and error messages. -// For usernames <= 4 characters, returns "****" -// For longer usernames, shows first 2 and last 2 characters with **** in between -// Useful for email addresses in authentication contexts. -// -// Examples: -// - "abc" -> "****" -// - "user@example.com" -> "us****om" -// - "admin" -> "ad****in" -func maskUsername(username string) string { - if len(username) <= 4 { - return "****" - } - // Show first 2 and last 2 characters - return username[:2] + "****" + username[len(username)-2:] -} - -// maskAccessToken masks an OAuth2 access token for display in logs. -// For tokens <= 16 characters, returns "****" -// For longer tokens, shows first 8 and last 4 characters with ... in between -// Access tokens are typically longer than passwords, so we show more context. -// -// Examples: -// - "short" -> "****" -// - "ya29.a0AfH6SMBxyz123456789abcdef" -> "ya29.a0A...cdef" -func maskAccessToken(token string) string { - if len(token) <= 16 { - return "****" - } - return token[:8] + "..." + token[len(token)-4:] -} diff --git a/internal/protocols/smtp/utils_test.go b/internal/protocols/smtp/utils_test.go deleted file mode 100644 index 5f818ea..0000000 --- a/internal/protocols/smtp/utils_test.go +++ /dev/null @@ -1,269 +0,0 @@ -//go:build !integration -// +build !integration - -package smtp - -import ( - "testing" -) - -// TestMaskPassword tests password masking for security (prevents password exposure in logs) -func TestMaskPassword(t *testing.T) { - tests := []struct { - name string - password string - expected string - }{ - // Short passwords (<= 8 chars) - fully masked, since revealing 4 - // characters of a password that short discloses most or all of it - {"Empty password", "", "****"}, - {"Single char", "a", "****"}, - {"Two chars", "ab", "****"}, - {"Three chars", "abc", "****"}, - {"Four chars", "1234", "****"}, - {"Five chars", "12345", "****"}, - {"Short password", "password", "****"}, - {"Mixed case", "MyP@ss", "****"}, - - // Longer passwords (> 8 chars) - show first 2 and last 2 - {"Longer password", "MySecretPassword", "My****rd"}, - {"Complex password", "P@ssw0rd!123", "P@****23"}, - {"Very long password", "ThisIsAVeryLongPasswordWithManyCharacters", "Th****rs"}, - - // Edge cases - {"Special characters", "!@#$%^&*()", "!@****()"}, - {"Unicode characters", "пароль123", "п****23"}, // Note: UTF-8 bytes, not runes - {"Spaces in password", "my password", "my****rd"}, - {"Numbers only", "123456789", "12****89"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := maskPassword(tt.password) - if result != tt.expected { - t.Errorf("maskPassword(%q) = %q, want %q", tt.password, result, tt.expected) - } - - // Security verification: Ensure original password is NOT in masked output (except for <= 4 chars) - if len(tt.password) > 4 && result == tt.password { - t.Errorf("maskPassword(%q) = %q, password not masked!", tt.password, result) - } - - // Security verification: Ensure masked output is not empty - if result == "" { - t.Errorf("maskPassword(%q) returned empty string", tt.password) - } - }) - } -} - -// TestMaskUsername tests username masking for security (prevents username exposure in logs) -func TestMaskUsername(t *testing.T) { - tests := []struct { - name string - username string - expected string - }{ - // Short usernames (<= 4 chars) - fully masked - {"Empty username", "", "****"}, - {"Single char", "a", "****"}, - {"Two chars", "ab", "****"}, - {"Three chars", "abc", "****"}, - {"Four chars", "user", "****"}, - - // Normal usernames (> 4 chars) - show first 2 and last 2 - {"Five chars", "admin", "ad****in"}, - {"Email address", "user@example.com", "us****om"}, - {"Long email", "firstname.lastname@company.com", "fi****om"}, - {"Simple username", "administrator", "ad****or"}, - {"Username with numbers", "user12345", "us****45"}, - - // Edge cases - {"Hyphenated username", "john-doe", "jo****oe"}, - {"Underscore username", "john_doe", "jo****oe"}, - {"Domain username", "DOMAIN\\username", "DO****me"}, - {"UPN format", "user@DOMAIN.LOCAL", "us****AL"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := maskUsername(tt.username) - if result != tt.expected { - t.Errorf("maskUsername(%q) = %q, want %q", tt.username, result, tt.expected) - } - - // Security verification: Ensure original username is NOT in masked output (except for <= 4 chars) - if len(tt.username) > 4 && result == tt.username { - t.Errorf("maskUsername(%q) = %q, username not masked!", tt.username, result) - } - - // Security verification: Ensure masked output is not empty - if result == "" { - t.Errorf("maskUsername(%q) returned empty string", tt.username) - } - }) - } -} - -// TestMaskPassword_SecurityProperties tests security properties of password masking -func TestMaskPassword_SecurityProperties(t *testing.T) { - t.Run("Masks all common passwords", func(t *testing.T) { - commonPasswords := []string{ - "password123", - "admin", - "letmein", - "welcome", - "monkey", - "dragon", - "master", - "qwerty", - "abc123", - "password", - } - - for _, password := range commonPasswords { - masked := maskPassword(password) - // Ensure password is not fully visible - if len(password) > 4 && masked == password { - t.Errorf("maskPassword() did not mask common password: %s", password) - } - // Ensure masking is consistent - masked2 := maskPassword(password) - if masked != masked2 { - t.Errorf("maskPassword() inconsistent for %s: %s != %s", password, masked, masked2) - } - } - }) - - t.Run("Masked output always contains asterisks", func(t *testing.T) { - testPasswords := []string{"", "a", "abc", "12345", "password123", "VeryLongPassword"} - for _, password := range testPasswords { - masked := maskPassword(password) - // All masked outputs should contain **** - if len(masked) < 4 { - t.Errorf("maskPassword(%q) output too short: %s", password, masked) - } - } - }) -} - -// TestMaskUsername_SecurityProperties tests security properties of username masking -func TestMaskUsername_SecurityProperties(t *testing.T) { - t.Run("Masks email addresses properly", func(t *testing.T) { - emails := []string{ - "admin@company.com", - "user@example.org", - "test.user@subdomain.example.com", - "firstname.lastname@corporate.example.net", - } - - for _, email := range emails { - masked := maskUsername(email) - // Ensure email is not fully visible - if masked == email { - t.Errorf("maskUsername() did not mask email: %s", email) - } - // Ensure @ symbol is hidden (it should be in the middle part) - // For long emails, @ will be masked - if len(masked) > 10 && masked[2] == '@' { - t.Errorf("maskUsername() exposed @ symbol for: %s -> %s", email, masked) - } - } - }) -} - -// TestMaskAccessToken tests OAuth2 access token masking for security -func TestMaskAccessToken(t *testing.T) { - tests := []struct { - name string - token string - expected string - }{ - // Short tokens (<= 16 chars) - fully masked - {"Empty token", "", "****"}, - {"Single char", "a", "****"}, - {"Short token", "abc123", "****"}, - {"Exactly 16 chars", "1234567890123456", "****"}, - - // Normal tokens (> 16 chars) - show first 8 and last 4 - {"17 chars", "12345678901234567", "12345678...4567"}, - {"Gmail-like token", "ya29.a0AfH6SMBxyz123456789abcdef", "ya29.a0A...cdef"}, - {"Azure AD token", "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6Ik", "eyJ0eXAi...I6Ik"}, - {"Long token", "ya29.a0AfH6SMBxyz123456789abcdefghijklmnopqrstuvwxyz", "ya29.a0A...wxyz"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := maskAccessToken(tt.token) - if result != tt.expected { - t.Errorf("maskAccessToken(%q) = %q, want %q", tt.token, result, tt.expected) - } - - // Security verification: Ensure original token is NOT in masked output (except for <= 16 chars) - if len(tt.token) > 16 && result == tt.token { - t.Errorf("maskAccessToken(%q) = %q, token not masked!", tt.token, result) - } - - // Security verification: Ensure masked output is not empty - if result == "" { - t.Errorf("maskAccessToken(%q) returned empty string", tt.token) - } - }) - } -} - -// TestMaskAccessToken_SecurityProperties tests security properties of access token masking -func TestMaskAccessToken_SecurityProperties(t *testing.T) { - t.Run("Masks real-world token formats", func(t *testing.T) { - realTokens := []string{ - "ya29.a0AfH6SMBxyz123456789abcdefghijklmnop", // Google OAuth2 - "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6IkN0VHVoTUpta", // Azure AD JWT - "gho_16C7e42F292c6912E7710c838347Ae178B4a", // GitHub - "xoxb-FAKE-TOKEN-FOR-TESTING-ONLY-NotARealToken123", // Slack-like format (fake) - } - - for _, token := range realTokens { - masked := maskAccessToken(token) - // Ensure token is not fully visible - if masked == token { - t.Errorf("maskAccessToken() did not mask token: %s", token) - } - // Ensure middle portion is hidden - if len(masked) > 20 { - t.Errorf("maskAccessToken() exposed too much of token: %s -> %s", token, masked) - } - } - }) - - t.Run("Masked output always contains separator", func(t *testing.T) { - testTokens := []string{"", "a", "short", "exactly16charss!", "longerToken12345678"} - for _, token := range testTokens { - masked := maskAccessToken(token) - // Short tokens get ****, long tokens get ... - if len(token) <= 16 { - if masked != "****" { - t.Errorf("maskAccessToken(%q) should be '****', got %s", token, masked) - } - } else { - if !contains(masked, "...") { - t.Errorf("maskAccessToken(%q) should contain '...', got %s", token, masked) - } - } - } - }) -} - -// helper function for string contains check -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || - (len(s) > 0 && len(substr) > 0 && findSubstring(s, substr))) -} - -func findSubstring(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} From ccc2183cbd87b83922cb89d35d62f910ecc17fd5 Mon Sep 17 00:00:00 2001 From: Ziemek Borowski Date: Sun, 14 Jun 2026 10:31:29 +0200 Subject: [PATCH 2/5] exportmessages --- .github/workflows/build.yml | 10 -- TOOLS.md | 190 ++++++++++++++----------- docs/protocols/msgraph.md | 16 ++- internal/protocols/msgraph/cmd.go | 52 ++++++- internal/protocols/msgraph/config.go | 21 +++ internal/protocols/msgraph/handlers.go | 141 ++++++++++++++++++ internal/protocols/msgraph/utils.go | 22 +++ 7 files changed, 353 insertions(+), 99 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0d1be3d..8e67255 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -229,16 +229,6 @@ jobs: -mailbox "user@example.com" -action getinbox ``` - ### Tools Overview - - | Tool | Protocol | Ports | Auth Methods | - |------|----------|-------|--------------| - | smtptool | SMTP | 25, 587, 465 | PLAIN, LOGIN, CRAM-MD5, XOAUTH2 | - | imaptool | IMAP | 143, 993 | PLAIN, LOGIN, XOAUTH2 | - | pop3tool | POP3 | 110, 995 | USER/PASS, APOP, XOAUTH2 | - | jmaptool | JMAP | 443 | Basic, Bearer | - | msgraphtool | Graph API | 443 | Client Secret, Certificate, Bearer | - ### Documentation - Online: [GitHub Repository](https://github.com/${{ github.repository }}) env: diff --git a/TOOLS.md b/TOOLS.md index 8be3dee..18f2959 100644 --- a/TOOLS.md +++ b/TOOLS.md @@ -1,17 +1,26 @@ -# gomailtesttool Suite - Tool Comparison +# gomailtesttool Suite - Protocol Comparison -This document provides a comprehensive comparison of all tools in the gomailtesttool suite. +This document provides a comprehensive comparison of all protocols supported by the `gomailtest` unified CLI. + +> Since v3.1, `gomailtest` is a single binary. Every protocol is a subcommand, +> and every action is a sub-subcommand: +> +> ``` +> gomailtest [flags] +> ``` +> +> The legacy per-protocol binaries (`smtptool`, `imaptool`, `pop3tool`, `jmaptool`, `ewstool`, `msgraphtool`) were removed in v3.1.0. ## Quick Reference -| Tool | Protocol | Default Port | Primary Use Case | -|------|----------|--------------|------------------| -| **smtptool** | SMTP | 25/587/465 | Test SMTP servers, TLS, authentication | -| **imaptool** | IMAP | 143/993 | Test IMAP servers, list folders | -| **pop3tool** | POP3 | 110/995 | Test POP3 servers, list messages | -| **jmaptool** | JMAP | 443 | Test JMAP servers (modern email API) | -| **ewstool** | EWS | 443 | Test on-premises Exchange EWS (Exchange 2007–2019) | -| **msgraphtool** | Microsoft Graph | 443 | Exchange Online via Microsoft Graph API | +| Protocol | Subcommand | Standard Port | Primary Use Case | +|----------|------------|----------------|------------------| +| **SMTP** | `gomailtest smtp` | 25/587/465 | Test SMTP servers, TLS, authentication | +| **IMAP** | `gomailtest imap` | 143/993 | Test IMAP servers, list folders | +| **POP3** | `gomailtest pop3` | 110/995 | Test POP3 servers, list messages | +| **JMAP** | `gomailtest jmap` | 443 | Test JMAP servers (modern email API) | +| **EWS** | `gomailtest ews` | 443 | Test on-premises Exchange EWS (Exchange 2007–2019) | +| **Microsoft Graph** | `gomailtest msgraph` | 443 | Exchange Online via Microsoft Graph API | --- @@ -19,8 +28,8 @@ This document provides a comprehensive comparison of all tools in the gomailtest ### Protocol Support -| Feature | smtptool | imaptool | pop3tool | jmaptool | ewstool | msgraphtool | -|---------|----------|----------|----------|----------|---------|-------------| +| Feature | smtp | imap | pop3 | jmap | ews | msgraph | +|---------|------|------|------|------|-----|---------| | TCP Connection | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Implicit TLS (SSL) | ✅ (SMTPS) | ✅ (IMAPS) | ✅ (POP3S) | ✅ (HTTPS) | ✅ (HTTPS) | ✅ (HTTPS) | | STARTTLS | ✅ | ✅ | ✅ | N/A | N/A | N/A | @@ -30,8 +39,8 @@ This document provides a comprehensive comparison of all tools in the gomailtest ### Authentication Methods -| Method | smtptool | imaptool | pop3tool | jmaptool | ewstool | msgraphtool | -|--------|----------|----------|----------|----------|---------|-------------| +| Method | smtp | imap | pop3 | jmap | ews | msgraph | +|--------|------|------|------|------|-----|---------| | PLAIN | ✅ | ✅ | - | - | - | - | | LOGIN | ✅ | ✅ | - | - | - | - | | CRAM-MD5 | ✅ | - | - | - | - | - | @@ -49,8 +58,8 @@ This document provides a comprehensive comparison of all tools in the gomailtest ### Available Actions -| Action | smtptool | imaptool | pop3tool | jmaptool | ewstool | msgraphtool | -|--------|----------|----------|----------|----------|---------|-------------| +| Action | smtp | imap | pop3 | jmap | ews | msgraph | +|--------|------|------|------|------|-----|---------| | Test Connection | ✅ `testconnect` | ✅ `testconnect` | ✅ `testconnect` | ✅ `testconnect` | ✅ `testconnect` | - | | Test STARTTLS | ✅ `teststarttls` | - | - | - | - | - | | Test Authentication | ✅ `testauth` | ✅ `testauth` | ✅ `testauth` | ✅ `testauth` | ✅ `testauth` | - | @@ -63,6 +72,7 @@ This document provides a comprehensive comparison of all tools in the gomailtest | Send Invite | - | - | - | - | - | ✅ `sendinvite` | | Export Inbox | - | - | - | - | - | ✅ `exportinbox` | | Search & Export | - | - | - | - | - | ✅ `searchandexport` | +| Export Messages (.eml) | - | - | - | - | - | ✅ `exportmessages` | | Get Folder | - | - | - | - | ✅ `getfolder` | - | | Autodiscover | - | - | - | - | ✅ `autodiscover` | - | @@ -86,16 +96,16 @@ This document provides a comprehensive comparison of all tools in the gomailtest ### Naming Convention -All tools use a consistent naming pattern: `{TOOL}{PARAMETER}` (no underscores) +Each protocol's flags can be set via environment variables using a consistent prefix: `{PREFIX}{PARAMETER}` (no underscores) -| Tool | Prefix | Example | -|------|--------|---------| -| smtptool | `SMTP` | `SMTPHOST`, `SMTPPORT`, `SMTPUSERNAME` | -| imaptool | `IMAP` | `IMAPHOST`, `IMAPPORT`, `IMAPUSERNAME` | -| pop3tool | `POP3` | `POP3HOST`, `POP3PORT`, `POP3USERNAME` | -| jmaptool | `JMAP` | `JMAPHOST`, `JMAPPORT`, `JMAPUSERNAME` | -| ewstool | `EWS` | `EWSHOST`, `EWSPORT`, `EWSUSERNAME` | -| msgraphtool | `MSGRAPH` | `MSGRAPHTENANTID`, `MSGRAPHCLIENTID` | +| Protocol | Prefix | Example | +|----------|--------|---------| +| smtp | `SMTP` | `SMTPHOST`, `SMTPPORT`, `SMTPUSERNAME` | +| imap | `IMAP` | `IMAPHOST`, `IMAPPORT`, `IMAPUSERNAME` | +| pop3 | `POP3` | `POP3HOST`, `POP3PORT`, `POP3USERNAME` | +| jmap | `JMAP` | `JMAPHOST`, `JMAPPORT`, `JMAPUSERNAME` | +| ews | `EWS` | `EWSHOST`, `EWSPORT`, `EWSUSERNAME` | +| msgraph | `MSGRAPH` | `MSGRAPHTENANTID`, `MSGRAPHCLIENTID` | ### Common Environment Variables @@ -107,23 +117,26 @@ All tools use a consistent naming pattern: `{TOOL}{PARAMETER}` (no underscores) | `{PREFIX}USERNAME` | Username for authentication | | `{PREFIX}PASSWORD` | Password for authentication | | `{PREFIX}ACCESSTOKEN` | OAuth2/Bearer access token | -| `{PREFIX}ACTION` | Action to perform | | `{PREFIX}VERBOSE` | Enable verbose output | | `{PREFIX}LOGLEVEL` | Log level (debug, info, warn, error) | | `{PREFIX}LOGFORMAT` | Log format (csv, json) | +> Note: there is no `{PREFIX}ACTION` variable. The action is selected by the +> subcommand you run (e.g. `gomailtest smtp testconnect`), not by a flag or +> environment variable. + --- ## Output Formats -All tools support: +All protocols support: - **Console Output**: Human-readable status and results - **CSV Logging**: Structured logs for analysis - **JSON Logging**: Machine-readable logs ### CSV Log Files -Log files are created with the pattern: `_{tool}_{action}_{date}.csv` +Log files are created with the pattern: `_{protocol}tool_{action}_{date}.csv` Example: `_smtptool_testconnect_20260131.csv` @@ -135,70 +148,71 @@ Example: `_smtptool_testconnect_20260131.csv` ```bash # Test basic connectivity -./smtptool -action testconnect -host smtp.example.com -port 25 +gomailtest smtp testconnect --host smtp.example.com --port 25 -# Test STARTTLS upgrade -./smtptool -action teststarttls -host smtp.example.com -port 587 +# Test STARTTLS / TLS diagnostics +gomailtest smtp teststarttls --host smtp.example.com --port 587 # Test authentication -./smtptool -action testauth -host smtp.example.com -port 587 \ - -username user@example.com -password "secret" -starttls +gomailtest smtp testauth --host smtp.example.com --port 587 \ + --username user@example.com --password "secret" # Send test email -./smtptool -action sendmail -host smtp.example.com -port 587 \ - -username user@example.com -password "secret" -starttls \ - -to recipient@example.com -subject "Test" -body "Hello" +gomailtest smtp sendmail --host smtp.example.com --port 587 \ + --username user@example.com --password "secret" \ + --from sender@example.com --to recipient@example.com \ + --subject "Test" --body "Hello" ``` ### IMAP Testing ```bash # Test connection with IMAPS -./imaptool -action testconnect -host imap.gmail.com -imaps +gomailtest imap testconnect --host imap.gmail.com --port 993 --imaps # Test authentication -./imaptool -action testauth -host imap.gmail.com -imaps \ - -username user@gmail.com -password "app-password" +gomailtest imap testauth --host imap.gmail.com --port 993 --imaps \ + --username user@gmail.com --password "app-password" # List folders with OAuth2 -./imaptool -action listfolders -host imap.gmail.com -imaps \ - -username user@gmail.com -accesstoken "ya29..." +gomailtest imap listfolders --host imap.gmail.com --port 993 --imaps \ + --username user@gmail.com --accesstoken "ya29..." ``` ### POP3 Testing ```bash # Test connection with POP3S -./pop3tool -action testconnect -host pop.gmail.com -pop3s +gomailtest pop3 testconnect --host pop.gmail.com --port 995 --pop3s # Test authentication -./pop3tool -action testauth -host pop.gmail.com -pop3s \ - -username user@gmail.com -password "app-password" +gomailtest pop3 testauth --host pop.gmail.com --port 995 --pop3s \ + --username user@gmail.com --password "app-password" -# List messages -./pop3tool -action listmail -host pop.gmail.com -pop3s \ - -username user@gmail.com -accesstoken "ya29..." +# List messages with OAuth2 +gomailtest pop3 listmail --host pop.gmail.com --port 995 --pop3s \ + --username user@gmail.com --accesstoken "ya29..." ``` ### JMAP Testing ```bash # Test JMAP session discovery -./jmaptool -action testconnect -host jmap.fastmail.com +gomailtest jmap testconnect --host jmap.fastmail.com # Test authentication with Bearer token -./jmaptool -action testauth -host jmap.fastmail.com \ - -username user@fastmail.com -accesstoken "fmu1-..." +gomailtest jmap testauth --host jmap.fastmail.com \ + --username user@fastmail.com --accesstoken "fmu1-..." # Get mailboxes -./jmaptool -action getmailboxes -host jmap.fastmail.com \ - -username user@fastmail.com -accesstoken "fmu1-..." +gomailtest jmap getmailboxes --host jmap.fastmail.com \ + --username user@fastmail.com --accesstoken "fmu1-..." ``` ### EWS Testing ```bash -# Test EWS connectivity +# Test EWS connectivity (no credentials required) gomailtest ews testconnect --host mail.example.com # Test NTLM authentication @@ -218,45 +232,47 @@ gomailtest ews autodiscover --host mail.example.com \ ```bash # Get inbox messages -./msgraphtool -tenantid "..." -clientid "..." -secret "..." \ - -mailbox "user@example.com" -action getinbox +gomailtest msgraph getinbox --tenantid "..." --clientid "..." --secret "..." \ + --mailbox "user@example.com" # Send email -./msgraphtool -tenantid "..." -clientid "..." -secret "..." \ - -mailbox "user@example.com" -action sendmail \ - -to "recipient@example.com" -subject "Test" -body "Hello" +gomailtest msgraph sendmail --tenantid "..." --clientid "..." --secret "..." \ + --mailbox "user@example.com" \ + --to "recipient@example.com" --subject "Test" --body "Hello" -# Using Bearer token -./msgraphtool -bearertoken "eyJ0..." \ - -mailbox "user@example.com" -action getinbox +# Export messages matching a subject as .eml (using a Bearer token) +gomailtest msgraph exportmessages --bearertoken "eyJ0..." \ + --mailbox "user@example.com" --subject "Invoice" ``` --- -## Choosing the Right Tool - -| Scenario | Recommended Tool | -|----------|------------------| -| Testing on-premises Exchange/SMTP | smtptool | -| Testing on-premises Exchange EWS | ewstool | -| Testing Gmail/Office 365 IMAP | imaptool | -| Testing legacy POP3 servers | pop3tool | -| Testing modern email providers (Fastmail) | jmaptool | -| Testing Exchange Online | msgraphtool | -| TLS/SSL diagnostics | smtptool (best TLS analysis) | -| OAuth2/XOAUTH2 testing | imaptool, pop3tool | -| Bulk mailbox operations | msgraphtool | -| Autodiscover troubleshooting | ewstool | +## Choosing the Right Protocol + +| Scenario | Recommended Subcommand | +|----------|------------------------| +| Testing on-premises Exchange/SMTP | `gomailtest smtp` | +| Testing on-premises Exchange EWS | `gomailtest ews` | +| Testing Gmail/Office 365 IMAP | `gomailtest imap` | +| Testing legacy POP3 servers | `gomailtest pop3` | +| Testing modern email providers (Fastmail) | `gomailtest jmap` | +| Testing Exchange Online | `gomailtest msgraph` | +| TLS/SSL diagnostics | `gomailtest smtp teststarttls` (best TLS analysis) | +| OAuth2/XOAUTH2 testing | `gomailtest imap`, `gomailtest pop3` | +| Bulk mailbox operations | `gomailtest msgraph` | +| Autodiscover troubleshooting | `gomailtest ews autodiscover` | --- ## Platform Support -| Platform | Architecture | File | -|----------|--------------|------| -| Windows | amd64 | `gomailtesttool-windows-amd64.zip` | -| Linux | amd64 | `gomailtesttool-linux-amd64.zip` | -| macOS | arm64 (Apple Silicon) | `gomailtesttool-macos-arm64.zip` | +All platforms build a single `gomailtest` binary (see [BUILD.md](BUILD.md) for cross-platform build commands). + +| Platform | Architecture | Binary | +|----------|--------------|--------| +| Windows | amd64 | `gomailtest.exe` | +| Linux | amd64 | `gomailtest` | +| macOS | amd64 / arm64 (Apple Silicon) | `gomailtest` | --- @@ -266,11 +282,11 @@ gomailtest ews autodiscover --host mail.example.com \ - [BUILD.md](BUILD.md) - Build instructions - [docs/protocols/msgraph.md](docs/protocols/msgraph.md) - Microsoft Graph examples and reference -### Tool-Specific Documentation +### Protocol-Specific Documentation -- [docs/protocols/ews.md](docs/protocols/ews.md) - EWS tool documentation -- [docs/protocols/msgraph.md](docs/protocols/msgraph.md) - Microsoft Graph tool documentation -- [docs/protocols/smtp.md](docs/protocols/smtp.md) - SMTP tool documentation -- [docs/protocols/imap.md](docs/protocols/imap.md) - IMAP tool documentation -- [docs/protocols/pop3.md](docs/protocols/pop3.md) - POP3 tool documentation -- [docs/protocols/jmap.md](docs/protocols/jmap.md) - JMAP tool documentation +- [docs/protocols/ews.md](docs/protocols/ews.md) - EWS protocol documentation +- [docs/protocols/msgraph.md](docs/protocols/msgraph.md) - Microsoft Graph protocol documentation +- [docs/protocols/smtp.md](docs/protocols/smtp.md) - SMTP protocol documentation +- [docs/protocols/imap.md](docs/protocols/imap.md) - IMAP protocol documentation +- [docs/protocols/pop3.md](docs/protocols/pop3.md) - POP3 protocol documentation +- [docs/protocols/jmap.md](docs/protocols/jmap.md) - JMAP protocol documentation diff --git a/docs/protocols/msgraph.md b/docs/protocols/msgraph.md index 1b4029a..e22db31 100644 --- a/docs/protocols/msgraph.md +++ b/docs/protocols/msgraph.md @@ -127,6 +127,20 @@ Output goes to `%TEMP%\export\{date}\message_{n}_{timestamp}.json`. gomailtest msgraph searchandexport --messageid "" ``` +### exportmessages — Export Matching Messages as .eml + +Searches messages by Internet Message-ID and/or a subject substring (OData +`contains()`), then downloads each match's raw RFC822 content as a `.eml` +file. + +```powershell +gomailtest msgraph exportmessages --subject "Invoice" +gomailtest msgraph exportmessages --messageid "" +gomailtest msgraph exportmessages --subject "Invoice" --count 10 +``` + +Output goes to `%TEMP%\export\{date}\msg_{id}.eml`. + ## Flags ### Persistent (all subcommands) @@ -285,7 +299,7 @@ Retry uses exponential backoff: 2s → 4s → 8s → 16s → 30s (capped). Retri |--------|-----------| | sendmail | `Mail.Send` | | getevents, sendinvite | `Calendars.ReadWrite` | -| getinbox, exportinbox, searchandexport | `Mail.Read` | +| getinbox, exportinbox, searchandexport, exportmessages | `Mail.Read` | | getschedule | `Calendars.Read` | ## Tips and Best Practices diff --git a/internal/protocols/msgraph/cmd.go b/internal/protocols/msgraph/cmd.go index ed0c88c..72f6c33 100644 --- a/internal/protocols/msgraph/cmd.go +++ b/internal/protocols/msgraph/cmd.go @@ -9,7 +9,7 @@ import ( "github.com/ziembor/gomailtesttool/internal/common/bootstrap" ) -// NewCmd returns the "msgraph" cobra.Command with all 7 action subcommands. +// NewCmd returns the "msgraph" cobra.Command with all 8 action subcommands. // Each subcommand shares persistent flags (auth, mailbox, output) and adds // its own action-specific flags. func NewCmd() *cobra.Command { @@ -38,6 +38,7 @@ Authentication methods: --secret (client secret), --pfx (certificate file), newGetScheduleCmd(v), newExportInboxCmd(v), newSearchAndExportCmd(v), + newExportMessagesCmd(v), ) return cmd @@ -408,3 +409,52 @@ func newSearchAndExportCmd(v *viper.Viper) *cobra.Command { cmd.Flags().String("messageid", "", "Internet Message-ID to search for (env: MSGRAPHMESSAGEID)") return cmd } + +func newExportMessagesCmd(v *viper.Viper) *cobra.Command { + cmd := &cobra.Command{ + Use: "exportmessages", + Short: "Search messages by Message-ID and/or Subject and export them as .eml files", + RunE: func(cmd *cobra.Command, args []string) error { + _ = v.BindPFlags(cmd.Flags()) + _ = v.BindPFlags(cmd.InheritedFlags()) + + if err := bootstrap.LoadConfigFile(v, v.GetString("config")); err != nil { + return err + } + + config := ConfigFromViper(v) + config.Action = ActionExportMessages + + if err := validateConfiguration(config); err != nil { + return fmt.Errorf("validation failed: %w", err) + } + + ctx, cancel := bootstrap.SetupSignalContext() + defer cancel() + + slogger, csvLogger, logErr := bootstrap.InitLoggers("msgraphtool", ActionExportMessages, config.VerboseMode, config.LogLevel, config.LogFormat) + if logErr != nil { + slogger.Warn("Could not initialize file logging", "error", logErr) + } + if csvLogger != nil { + defer csvLogger.Close() + } + + if config.ProxyURL != "" { + os.Setenv("HTTP_PROXY", config.ProxyURL) + os.Setenv("HTTPS_PROXY", config.ProxyURL) + } + + client, err := NewGraphServiceClient(ctx, config, slogger) + if err != nil { + return err + } + + return exportMessages(ctx, client, config.Mailbox, config.MessageID, config.Subject, config.Count, config, csvLogger) + }, + } + cmd.Flags().String("messageid", "", "Internet Message-ID to search for (env: MSGRAPHMESSAGEID)") + cmd.Flags().String("subject", "", "Subject substring to search for, used with OData contains() (env: MSGRAPHSUBJECT)") + cmd.Flags().Int("count", 25, "Maximum number of matching messages to export (env: MSGRAPHCOUNT)") + return cmd +} diff --git a/internal/protocols/msgraph/config.go b/internal/protocols/msgraph/config.go index 0a80302..355f50f 100644 --- a/internal/protocols/msgraph/config.go +++ b/internal/protocols/msgraph/config.go @@ -92,6 +92,7 @@ const ( ActionGetSchedule = "getschedule" ActionExportInbox = "exportinbox" ActionSearchAndExport = "searchandexport" + ActionExportMessages = "exportmessages" ) // RegisterPersistentFlags registers flags shared by all msgraph subcommands @@ -385,6 +386,26 @@ func validateConfiguration(config *Config) error { } } + // Validate exportmessages-specific requirements + if config.Action == ActionExportMessages { + if config.MessageID == "" && strings.TrimSpace(config.Subject) == "" { + return fmt.Errorf("exportmessages action requires --messageid and/or --subject parameter") + } + + if config.MessageID != "" { + // SECURITY: Validate Message-ID format to prevent OData injection attacks + if err := validateMessageID(config.MessageID); err != nil { + return fmt.Errorf("invalid message ID: %w", err) + } + } + + if config.Subject != "" { + if err := validateSearchSubject(config.Subject); err != nil { + return fmt.Errorf("invalid subject: %w", err) + } + } + } + return nil } diff --git a/internal/protocols/msgraph/handlers.go b/internal/protocols/msgraph/handlers.go index 18958c0..1d1f5b2 100644 --- a/internal/protocols/msgraph/handlers.go +++ b/internal/protocols/msgraph/handlers.go @@ -699,6 +699,147 @@ func searchAndExport(ctx context.Context, client *msgraphsdk.GraphServiceClient, return nil } +// exportMessages searches for messages matching the given Internet Message-ID +// and/or subject pattern and exports each match as a raw .eml (RFC822) file. +func exportMessages(ctx context.Context, client *msgraphsdk.GraphServiceClient, mailbox string, messageID string, subject string, count int, config *Config, logger logger.Logger) error { + // Build OData filter from the supplied criteria. + // SECURITY: Escape single quotes for OData filter (defense-in-depth); + // validateMessageID()/validateSearchSubject() already reject control + // characters and (for messageID) quotes. + var clauses []string + if messageID != "" { + escapedMessageID := strings.ReplaceAll(messageID, "'", "''") + clauses = append(clauses, fmt.Sprintf("internetMessageId eq '%s'", escapedMessageID)) + } + if subject != "" { + escapedSubject := strings.ReplaceAll(subject, "'", "''") + clauses = append(clauses, fmt.Sprintf("contains(subject,'%s')", escapedSubject)) + } + filter := strings.Join(clauses, " and ") + + requestConfig := &users.ItemMessagesRequestBuilderGetRequestConfiguration{ + QueryParameters: &users.ItemMessagesRequestBuilderGetQueryParameters{ + Filter: &filter, + Top: Int32Ptr(int32(count)), + Select: []string{"id", "internetMessageId", "subject", "receivedDateTime", "from", "toRecipients", "ccRecipients", "bccRecipients", "hasAttachments"}, + }, + } + + logVerbose(config.VerboseMode, "Calling Graph API: GET /users/%s/messages?$filter=%s&$top=%d", mailbox, filter, count) + + // Execute API call with retry logic + var getValueFunc func() []models.Messageable + err := retryWithBackoff(ctx, config.MaxRetries, config.RetryDelay, func() error { + apiResult, apiErr := client.Users().ByUserId(mailbox).Messages().Get(ctx, requestConfig) + if apiErr == nil { + getValueFunc = apiResult.GetValue + } + return apiErr + }) + + if err != nil { + enrichedErr := enrichGraphAPIError(err, logger, "exportMessages") + return fmt.Errorf("error searching messages for %s: %w", mailbox, enrichedErr) + } + + messages := getValueFunc() + messageCount := len(messages) + + logVerbose(config.VerboseMode, "API response received: %d messages", messageCount) + + if messageCount == 0 { + if config.OutputFormat == "json" { + printJSON([]interface{}{}) // Empty array + } else { + fmt.Println("No messages found matching the given criteria.") + } + if logger != nil { + _ = logger.WriteRow([]string{ActionExportMessages, StatusSuccess, mailbox, "No messages found (0 messages)", "N/A"}) + } + return nil + } + + // Print JSON output if requested + if config.OutputFormat == "json" { + printJSON(formatMessagesOutput(messages)) + } + + // Create export directory + exportDir, err := createExportDir() + if err != nil { + return err + } + + if config.OutputFormat != "json" { + fmt.Printf("Export directory: %s\n", exportDir) + } + + successCount := 0 + for _, message := range messages { + filePath, err := exportMessageToEML(ctx, client, mailbox, message, exportDir, config) + if err != nil { + log.Printf("Error exporting message ID %s: %v", *message.GetId(), err) + if logger != nil { + _ = logger.WriteRow([]string{ActionExportMessages, StatusError, mailbox, err.Error(), *message.GetId()}) + } + continue + } + + successCount++ + if config.OutputFormat != "json" { + fmt.Printf("Successfully exported message: %s -> %s\n", *message.GetSubject(), filePath) + } + if logger != nil { + _ = logger.WriteRow([]string{ActionExportMessages, StatusSuccess, mailbox, "Exported successfully", *message.GetId(), filePath}) + } + } + + if config.OutputFormat != "json" { + fmt.Printf("Successfully exported %d/%d messages.\n", successCount, messageCount) + } + + return nil +} + +// exportMessageToEML fetches a message's raw RFC822/MIME content via the +// Graph API "$value" endpoint and writes it to a .eml file in dir. It returns +// the path to the written file. +func exportMessageToEML(ctx context.Context, client *msgraphsdk.GraphServiceClient, mailbox string, message models.Messageable, dir string, config *Config) (string, error) { + if message.GetId() == nil { + return "", fmt.Errorf("message has no ID") + } + id := *message.GetId() + + var mimeContent []byte + err := retryWithBackoff(ctx, config.MaxRetries, config.RetryDelay, func() error { + content, apiErr := client.Users().ByUserId(mailbox).Messages().ByMessageId(id).Content().Get(ctx, nil) + if apiErr == nil { + mimeContent = content + } + return apiErr + }) + if err != nil { + return "", fmt.Errorf("failed to fetch message content: %w", err) + } + + // Prefer the Internet Message-ID for the filename when available, since + // it's more useful for cross-referencing than the Graph-internal ID. + name := id + if message.GetInternetMessageId() != nil && *message.GetInternetMessageId() != "" { + name = *message.GetInternetMessageId() + } + + filename := fmt.Sprintf("msg_%s.eml", sanitizeFilename(name)) + filePath := filepath.Join(dir, filename) + + if err := os.WriteFile(filePath, mimeContent, 0644); err != nil { + return "", fmt.Errorf("failed to write EML file: %w", err) + } + + logVerbose(config.VerboseMode, "Exported message to %s", filePath) + return filePath, nil +} + // exportMessageToJSON serializes a message to JSON and saves it to a file func exportMessageToJSON(message models.Messageable, dir string, config *Config) error { // Extract basic info for filename diff --git a/internal/protocols/msgraph/utils.go b/internal/protocols/msgraph/utils.go index ba8cd4c..ce4c47a 100644 --- a/internal/protocols/msgraph/utils.go +++ b/internal/protocols/msgraph/utils.go @@ -104,6 +104,28 @@ func validateMessageID(msgID string) error { return nil } +// validateSearchSubject validates a subject search pattern used in an OData +// contains() filter. Single quotes are escaped (doubled) by the caller before +// being embedded in the filter, so this only rejects empty/oversized input and +// control characters that have no place in an email subject. +func validateSearchSubject(subject string) error { + if strings.TrimSpace(subject) == "" { + return fmt.Errorf("subject cannot be empty") + } + + if len(subject) > 998 { + return fmt.Errorf("exceeds maximum length of 998 characters") + } + + for _, r := range subject { + if r < 0x20 && r != '\t' { + return fmt.Errorf("contains invalid control characters") + } + } + + return nil +} + // enrichGraphAPIError enriches Graph API errors with additional context, // particularly for rate limiting scenarios. It detects rate limit errors (429) // and extracts the Retry-After header if available. From 9b611466e29c951910e18288041824aa3f3257ac Mon Sep 17 00:00:00 2001 From: Ziemek Borowski Date: Sun, 14 Jun 2026 11:05:10 +0200 Subject: [PATCH 3/5] ONBOARDING.md --- ONBOARDING.md | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 ONBOARDING.md diff --git a/ONBOARDING.md b/ONBOARDING.md new file mode 100644 index 0000000..6bdd8fd --- /dev/null +++ b/ONBOARDING.md @@ -0,0 +1,67 @@ +# Welcome to the Team + +## How We Use Claude + +Based on Ziemek Borowski's usage over the last 30 days: + +Work Type Breakdown: + Plan Design ██████░░░░░░░░░░░░░░ 30% + Write Docs ██████░░░░░░░░░░░░░░ 30% + Build Feature ████░░░░░░░░░░░░░░░░ 20% + Prototype ████░░░░░░░░░░░░░░░░ 20% + +Top Skills & Commands: + /ultrareview ████████████████████ 2x/month + /advisor ██████████░░░░░░░░░░ 1x/month + /remote-control ██████████░░░░░░░░░░ 1x/month + /usage ██████████░░░░░░░░░░ 1x/month + +Top MCP Servers: + (none used yet this period) + +## Your Setup Checklist + +### Codebases +- [ ] gomailtesttool — github.com/ziembor/gomailtesttool + +### MCP Servers to Activate +- [ ] Gmail — Read/send Gmail messages directly from Claude; useful for verifying mail delivered by gomailtesttool's send/test commands. Activate via `/mcp` and run `authenticate`/`complete_authentication`. +- [ ] Google Calendar — Inspect calendar invites sent via `msgraph sendinvite`/`getschedule`. Activate via `/mcp` and run `authenticate`/`complete_authentication`. +- [ ] Google Drive — Pull reference docs/specs into context if needed. Activate via `/mcp` and run `authenticate`/`complete_authentication`. + +### Skills to Know About +- `/ultrareview` — Launches a multi-agent cloud review of the current branch (or a GitHub PR with `/ultrareview `). Good for "review this whole change before I merge" moments. +- `/advisor` — Consults a stronger reviewer model with full context of your session; useful before committing to an approach or when stuck. +- `/remote-control` — Hands off a plan to Claude Code on the web (Ultraplan) so it can be refined/executed remotely while you keep working locally; results land as a PR. +- `/context` — Shows current context-window usage, handy when a session is getting long. +- `/usage` — Shows your Claude usage/limits. + +## Team Tips + +_TODO_ + +## Get Started + +_TODO_ + + From eff7fa56f50d0a9b0f475e6face4c887e51b875c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 09:17:14 +0000 Subject: [PATCH 4/5] Fix lint failures: add maskUsername to imap/pop3, remove duplicate newExportMessagesCmd in msgraph --- internal/protocols/imap/utils.go | 11 +++++++ internal/protocols/msgraph/cmd.go | 49 ------------------------------- internal/protocols/pop3/utils.go | 11 +++++++ 3 files changed, 22 insertions(+), 49 deletions(-) create mode 100644 internal/protocols/imap/utils.go create mode 100644 internal/protocols/pop3/utils.go diff --git a/internal/protocols/imap/utils.go b/internal/protocols/imap/utils.go new file mode 100644 index 0000000..6142eca --- /dev/null +++ b/internal/protocols/imap/utils.go @@ -0,0 +1,11 @@ +package imap + +// maskUsername masks usernames/emails for safe display in logs. +// Shows first 2 and last 2 characters with **** in between. +// Examples: "user@example.com" -> "us****om", "ab" -> "****" +func maskUsername(username string) string { + if len(username) <= 4 { + return "****" + } + return username[:2] + "****" + username[len(username)-2:] +} diff --git a/internal/protocols/msgraph/cmd.go b/internal/protocols/msgraph/cmd.go index 9b65478..251ec32 100644 --- a/internal/protocols/msgraph/cmd.go +++ b/internal/protocols/msgraph/cmd.go @@ -465,52 +465,3 @@ func newExportMessagesCmd(v *viper.Viper) *cobra.Command { cmd.Flags().String("exportdir", "", "Directory under which to create the dated export folder; defaults to the OS temp directory (env: MSGRAPHEXPORTDIR)") return cmd } - -func newExportMessagesCmd(v *viper.Viper) *cobra.Command { - cmd := &cobra.Command{ - Use: "exportmessages", - Short: "Search messages by Message-ID and/or Subject and export them as .eml files", - RunE: func(cmd *cobra.Command, args []string) error { - _ = v.BindPFlags(cmd.Flags()) - _ = v.BindPFlags(cmd.InheritedFlags()) - - if err := bootstrap.LoadConfigFile(v, v.GetString("config")); err != nil { - return err - } - - config := ConfigFromViper(v) - config.Action = ActionExportMessages - - if err := validateConfiguration(config); err != nil { - return fmt.Errorf("validation failed: %w", err) - } - - ctx, cancel := bootstrap.SetupSignalContext() - defer cancel() - - slogger, csvLogger, logErr := bootstrap.InitLoggers("msgraphtool", ActionExportMessages, config.VerboseMode, config.LogLevel, config.LogFormat) - if logErr != nil { - slogger.Warn("Could not initialize file logging", "error", logErr) - } - if csvLogger != nil { - defer csvLogger.Close() - } - - if config.ProxyURL != "" { - os.Setenv("HTTP_PROXY", config.ProxyURL) - os.Setenv("HTTPS_PROXY", config.ProxyURL) - } - - client, err := NewGraphServiceClient(ctx, config, slogger) - if err != nil { - return err - } - - return exportMessages(ctx, client, config.Mailbox, config.MessageID, config.Subject, config.Count, config, csvLogger) - }, - } - cmd.Flags().String("messageid", "", "Internet Message-ID to search for (env: MSGRAPHMESSAGEID)") - cmd.Flags().String("subject", "", "Subject substring to search for, used with OData contains() (env: MSGRAPHSUBJECT)") - cmd.Flags().Int("count", 25, "Maximum number of matching messages to export (env: MSGRAPHCOUNT)") - return cmd -} diff --git a/internal/protocols/pop3/utils.go b/internal/protocols/pop3/utils.go new file mode 100644 index 0000000..231e28f --- /dev/null +++ b/internal/protocols/pop3/utils.go @@ -0,0 +1,11 @@ +package pop3 + +// maskUsername masks usernames/emails for safe display in logs. +// Shows first 2 and last 2 characters with **** in between. +// Examples: "user@example.com" -> "us****om", "ab" -> "****" +func maskUsername(username string) string { + if len(username) <= 4 { + return "****" + } + return username[:2] + "****" + username[len(username)-2:] +} From 457f28b406e87ebf60046a6cf8dad45e97f5b0be Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 09:20:38 +0000 Subject: [PATCH 5/5] Fix merge conflict marker in docs/protocols/msgraph.md --- docs/protocols/msgraph.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/protocols/msgraph.md b/docs/protocols/msgraph.md index a1b2c2d..a93aa37 100644 --- a/docs/protocols/msgraph.md +++ b/docs/protocols/msgraph.md @@ -137,11 +137,6 @@ file. gomailtest msgraph exportmessages --subject "Invoice" gomailtest msgraph exportmessages --messageid "" gomailtest msgraph exportmessages --subject "Invoice" --count 10 - -``` - -Output goes to `%TEMP%\export\{date}\msg_{id}.eml`. -======= gomailtest msgraph exportmessages --subject "Invoice" --exportdir "C:\exports" ```