Skip to content

Address v3.2.0 EWS review findings: security hardening, dead-config cleanup, and targeted test coverage#55

Draft
Copilot wants to merge 4 commits intomainfrom
copilot/review-last-code-review
Draft

Address v3.2.0 EWS review findings: security hardening, dead-config cleanup, and targeted test coverage#55
Copilot wants to merge 4 commits intomainfrom
copilot/review-last-code-review

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 13, 2026

This PR implements the action plan from the latest v3.2.0 EWS review by resolving merge-blocking security/correctness issues and removing misleading EWS options that had no runtime effect. It also aligns migration docs with the implemented dependency stack and adds focused unit coverage for EWS config and TLS helpers.

  • Security and robustness fixes

    • Escaped Autodiscover email before XML interpolation to prevent malformed/injectable SOAP payloads.
    • Added bounded response reads in EWS HTTP paths (SendSOAP, SendAutodiscover) via io.LimitReader (4 MiB cap).
    • Replaced string-only fmt.Errorf("%s", ...) with errors.New(...) in EWS action error returns.
  • Correctness and UX

    • Removed custom min() helper from testauth.go (conflicted with Go builtin on modern Go versions).
    • Fixed folder ID display logic to append ... only when truncation actually occurs.
    • Moved shared SOAP request constant (getFolderSOAPBody) into soap_bodies.go for clearer ownership/reuse.
  • Dead configuration removal (EWS-specific)

    • Removed unused EWS config/flags/env bindings that were not implemented at runtime:
      • maxretries / retrydelay
      • output / OutputFormat
    • Updated EWS config construction and defaults accordingly.
  • Dependency and migration-doc alignment

    • Updated migration/3.2_MIGRATION_PLAN_EWS.md to reflect actual implementation (github.com/Azure/go-ntlmssp, not go-ews).
    • Upgraded github.com/Azure/go-ntlmssp to v0.1.0.
  • Targeted test additions

    • Added internal/protocols/ews/config_test.go covering:
      • validateConfiguration
      • resolveAuthMethod
      • ConfigFromViper
      • buildTLSConfig
      • certCSVFields

Example of the key hardening change:

var escapedEmail strings.Builder
if err := xml.EscapeText(&escapedEmail, []byte(email)); err != nil {
    return nil, fmt.Errorf("escaping email: %w", err)
}
payload := fmt.Sprintf(autodiscoverEnvelope, escapedEmail.String())

respBody, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodyBytes))

ziembor and others added 4 commits April 13, 2026 10:49
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Co-authored-by: ziembor <1870879+ziembor@users.noreply.github.com>
New subcommand: gomailtest ews
  - testconnect  — HTTP/TLS probe, reports cert chain and response code
  - testauth     — GetFolder(Inbox) to verify credentials
  - getfolder    — GetFolder(Inbox) with folder stats output
  - autodiscover — SOAP Autodiscover to resolve internal/external EWS URLs

Auth: NTLM (go-ntlmssp), Basic, Bearer (OAuth2), auto-detection
New dependency: github.com/Azure/go-ntlmssp

Migration plan: migration/3.2_MIGRATION_PLAN_EWS.md

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Co-authored-by: ziembor <1870879+ziembor@users.noreply.github.com>
…leanup, and targeted test coverage

Agent-Logs-Url: https://github.com/ziembor/gomailtesttool/sessions/9ff4a193-0feb-49dc-9194-a7253c2912df

Co-authored-by: ziembor <1870879+ziembor@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants