Surface HTTP error body + add -v to diagnose

src/ScepWright.Client/CommandRouter.cs

@@ -707,12 +707,20 @@ private static int RunDiagnose(string[] args, string data_root, TextWriter outpu
         ScepResult<System.Collections.Generic.IReadOnlyList<X509Certificate2>> ca;
         RecipientSelection selection;
         int idx;
+        int verbosity;
+        ConsoleTrace tracer;
         System.Collections.Generic.List<string> cap_warnings;
 
-        if (args.Length < 2) { output.WriteLine("usage: diagnose <serverId>"); return 2; }
-        if (!RejectUnknownFlags(args, output, System.Array.Empty<string>(), System.Array.Empty<string>())) { return 2; }
+        if (args.Length < 2) { output.WriteLine("usage: diagnose <serverId> [-v]"); return 2; }
+        if (!RejectUnknownFlags(args, output, System.Array.Empty<string>(), new[] { "-v" })) { return 2; }
         if (!BuildClient(args, args[1], data_root, output, out client, out stored)) { return 2; }
 
+        verbosity = CountFlag(args, "-v");
+        if (verbosity > 0) {
+            tracer = new ConsoleTrace(verbosity);
+            client.Trace += tracer.Handle;
+        }
+
         output.WriteLine($"diagnose {stored.Id}  ({stored.Url})");
         output.WriteLine();
 
@@ -1835,7 +1843,7 @@ public static string HelpTest(string? run_as = null) {
 
         return string.Join('\n', new[] {
             header,
-            "  diagnose <serverId>              (read-only health check: caps, CA/RA cert details, recipient verdict)",
+            "  diagnose <serverId> [-v]         (read-only health check: caps, CA/RA cert details, recipient verdict; -v traces the request URLs)",
             "  getcacaps <serverId>",
             "  getcacert <serverId> [-v]        (-v shows full CA/RA cert details)",
             "  getnextcacert <serverId>",

src/ScepWright.Core/ScepClient.cs

@@ -77,6 +77,7 @@ public ScepResult<ScepCapabilities> GetCaCaps() {
         string text;
 
         Emit(TraceLevel.Info, "GetCaCaps", "sending GetCACaps request");
+        Emit(TraceLevel.Debug, "GetCaCaps", $"GET {_transport.DescribeGet("GetCACaps", Server.CaIdentifier ?? string.Empty)}");
         raw = _transport.Get("GetCACaps", Server.CaIdentifier ?? string.Empty);
         if (!raw.IsOk) {
             return ScepResult<ScepCapabilities>.Fail(raw.Status, raw.Error);
@@ -92,6 +93,7 @@ public async Task<ScepResult<ScepCapabilities>> GetCaCapsAsync() {
         string text;
 
         Emit(TraceLevel.Info, "GetCaCaps", "sending GetCACaps request");
+        Emit(TraceLevel.Debug, "GetCaCaps", $"GET {_transport.DescribeGet("GetCACaps", Server.CaIdentifier ?? string.Empty)}");
         raw = await _transport.GetAsync("GetCACaps", Server.CaIdentifier ?? string.Empty).ConfigureAwait(false);
         if (!raw.IsOk) {
             return ScepResult<ScepCapabilities>.Fail(raw.Status, raw.Error);
@@ -112,6 +114,7 @@ public ScepResult<IReadOnlyList<X509Certificate2>> GetCaCert() {
         string error;
 
         Emit(TraceLevel.Info, "GetCaCert", "sending GetCACert request");
+        Emit(TraceLevel.Debug, "GetCaCert", $"GET {_transport.DescribeGet("GetCACert", Server.CaIdentifier ?? string.Empty)}");
         raw = _transport.Get("GetCACert", Server.CaIdentifier ?? string.Empty);
         if (!raw.IsOk) {
             return ScepResult<IReadOnlyList<X509Certificate2>>.Fail(raw.Status, raw.Error);
@@ -131,6 +134,7 @@ public async Task<ScepResult<IReadOnlyList<X509Certificate2>>> GetCaCertAsync()
         string error;
 
         Emit(TraceLevel.Info, "GetCaCert", "sending GetCACert request");
+        Emit(TraceLevel.Debug, "GetCaCert", $"GET {_transport.DescribeGet("GetCACert", Server.CaIdentifier ?? string.Empty)}");
         raw = await _transport.GetAsync("GetCACert", Server.CaIdentifier ?? string.Empty).ConfigureAwait(false);
         if (!raw.IsOk) {
             return ScepResult<IReadOnlyList<X509Certificate2>>.Fail(raw.Status, raw.Error);

src/ScepWright.Core/Transport/ScepHttpTransport.cs

@@ -25,6 +25,14 @@ private Uri BuildGetUri(string operation, string message) {
         return new Uri(_base_url + query);
     }
 
+    /// <summary>
+    /// Returns the fully-resolved GET URL for an operation, for diagnostic logging — so `diagnose -v`
+    /// can show exactly what was requested without the caller re-deriving the query string.
+    /// </summary>
+    public string DescribeGet(string operation, string message) {
+        return BuildGetUri(operation, message).ToString();
+    }
+
     /// <summary>Issues a GET for the given SCEP operation with the message in the query string.</summary>
     public async Task<ScepResult<byte[]>> GetAsync(string operation, string message) {
         HttpResponseMessage resp;
@@ -84,14 +92,56 @@ public static string DescribeHttpError(int status) {
         return $"HTTP {status}";
     }
 
+    /// <summary>
+    /// As <see cref="DescribeHttpError(int)"/>, but folds in a snippet of the server's response body.
+    /// A bare "HTTP 500" hides the real reason — for a 500 the cause is almost always in the body the
+    /// server returned (an exception string, a stack, a Venafi error). The snippet is whitespace-collapsed
+    /// and truncated so a multi-kilobyte HTML error page can't flood the console.
+    /// </summary>
+    public static string DescribeHttpError(int status, string body) {
+        string baseline;
+        string snippet;
+
+        baseline = DescribeHttpError(status);
+        snippet = Snippet(body);
+        if (snippet.Length == 0) { return baseline; }
+        return $"{baseline} — server said: {snippet}";
+    }
+
+    private const int BodySnippetMax = 200;
+
+    private static string Snippet(string body) {
+        string collapsed;
+
+        if (string.IsNullOrWhiteSpace(body)) { return string.Empty; }
+        collapsed = System.Text.RegularExpressions.Regex.Replace(body.Trim(), "\\s+", " ");
+        if (collapsed.Length > BodySnippetMax) { return collapsed.Substring(0, BodySnippetMax) + "…"; }
+        return collapsed;
+    }
+
     private static ScepResult<byte[]> Read(HttpResponseMessage resp) {
-        if (!resp.IsSuccessStatusCode) { return ScepResult<byte[]>.Fail(ScepClientResult.NetworkError, DescribeHttpError((int)resp.StatusCode)); }
+        byte[] body;
+
+        if (!resp.IsSuccessStatusCode) {
+            body = resp.Content.ReadAsByteArrayAsync().GetAwaiter().GetResult();
+            return ScepResult<byte[]>.Fail(ScepClientResult.NetworkError, DescribeHttpError((int)resp.StatusCode, Decode(body)));
+        }
         return ScepResult<byte[]>.Ok(resp.Content.ReadAsByteArrayAsync().GetAwaiter().GetResult());
     }
 
     private static async Task<ScepResult<byte[]>> ReadAsync(HttpResponseMessage resp) {
-        if (!resp.IsSuccessStatusCode) { return ScepResult<byte[]>.Fail(ScepClientResult.NetworkError, DescribeHttpError((int)resp.StatusCode)); }
+        byte[] body;
+
+        if (!resp.IsSuccessStatusCode) {
+            body = await resp.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
+            return ScepResult<byte[]>.Fail(ScepClientResult.NetworkError, DescribeHttpError((int)resp.StatusCode, Decode(body)));
+        }
         return ScepResult<byte[]>.Ok(await resp.Content.ReadAsByteArrayAsync().ConfigureAwait(false));
     }
 
+    private static string Decode(byte[] body) {
+        if (body is null || body.Length == 0) { return string.Empty; }
+        return System.Text.Encoding.UTF8.GetString(body);
+    }
+
 }

tests/ScepWright.Tests/CliRouterPhase2Tests.cs

@@ -615,6 +615,27 @@ public async Task Servers_suggest_warns_and_omits_enroll_lines_for_a_broken_sign
         Assert.Contains("diagnose", text);
     }
 
+    // `diagnose` must accept -v so an operator can see the resolved request URL and trace lines;
+    // it used to reject every flag, exiting 2 (usage error) on `diagnose <server> -v`.
+    [Fact]
+    public async Task Diagnose_accepts_verbose_flag() {
+        await using ScepServerApp server = await ScepServerApp.StartAsync();
+        string root;
+        StringWriter outw;
+        StringWriter diag;
+        int code;
+
+        root = Directory.CreateTempSubdirectory().FullName;
+        outw = new StringWriter();
+        CommandRouter.Run(new[] { "servers", "add", server.ScepUrl.ToString(), "--name", "fake" }, root, outw);
+
+        diag = new StringWriter();
+        code = CommandRouter.Run(new[] { "diagnose", "fake", "-v" }, root, diag);
+
+        Assert.Equal(0, code);
+        Assert.Contains("GetCACaps", diag.ToString());
+    }
+
     private static string FirstCertId(string root, string server) {
         return Path.GetFileName(Directory.GetDirectories(Path.Combine(root, "servers", server, "certificates"))[0]);
     }

tests/ScepWright.Tests/OrchestrationTests.cs

@@ -89,4 +89,28 @@ public async Task GetNewCertificateAsync_fetches_ca_cert_enrolls_stores_and_logs
         Assert.True(File.Exists(history_file), $"Expected history file at {history_file}");
         Assert.True(File.ReadAllLines(history_file).Length >= 1, "Expected at least one history record");
     }
+
+    // `diagnose -v` is useless if the operator can't see what was actually requested. GetCaCaps must
+    // emit a Debug trace carrying the fully-resolved request URL.
+    [Fact]
+    public void GetCaCaps_emits_a_debug_trace_with_the_resolved_request_url() {
+        BouncyCastleScepCrypto crypto;
+        CannedHandler handler;
+        ScepClient client;
+        System.Collections.Generic.List<ScepTraceEvent> events;
+
+        crypto = new BouncyCastleScepCrypto();
+        handler = new CannedHandler { Pki = System.Text.Encoding.ASCII.GetBytes("POSTPKIOperation\nSHA-256") };
+        Assert.Equal(ScepClientResult.Ok, ScepClient.Create(
+            new ServerConfig { Id = "fake", Url = new Uri("https://host/vedscep/") },
+            crypto, handler, out client, out _));
+
+        events = new System.Collections.Generic.List<ScepTraceEvent>();
+        client.Trace += events.Add;
+        client.GetCaCaps();
+
+        Assert.Contains(events, e => e.Level == TraceLevel.Debug
+            && e.Message.Contains("operation=GetCACaps")
+            && e.Message.Contains("https://host/vedscep/"));
+    }
 }

tests/ScepWright.Tests/TransportErrorTests.cs

@@ -1,10 +1,35 @@
+using System;
+using System.Net;
+using System.Net.Http;
+using System.Threading;
+using System.Threading.Tasks;
 using ScepWright.Core.Transport;
+using ScepWright.Crypto;
 using Xunit;
 
 namespace ScepWright.Tests;
 
 // A 404 alone tells a non-expert nothing; it almost always means the SCEP URL path is wrong.
+// A 500 alone is worse: the real reason lives in the response body the server returned, so we
+// must surface that body instead of discarding it.
 public sealed class TransportErrorTests {
+    private sealed class StatusBodyHandler : HttpMessageHandler {
+        public HttpStatusCode Status = HttpStatusCode.InternalServerError;
+        public string Body = string.Empty;
+
+        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken ct) {
+            return Task.FromResult(Build());
+        }
+
+        protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken ct) {
+            return Build();
+        }
+
+        private HttpResponseMessage Build() {
+            return new HttpResponseMessage(Status) { Content = new StringContent(Body) };
+        }
+    }
+
     [Fact]
     public void Http_404_message_hints_at_the_url_path() {
         string msg;
@@ -18,4 +43,65 @@ public void Http_404_message_hints_at_the_url_path() {
     public void Other_http_errors_stay_terse() {
         Assert.Equal("HTTP 500", ScepHttpTransport.DescribeHttpError(500));
     }
+
+    [Fact]
+    public void Http_error_includes_the_server_response_body() {
+        string msg;
+
+        msg = ScepHttpTransport.DescribeHttpError(500, "VedSCEP handler threw NullReferenceException");
+        Assert.Contains("500", msg);
+        Assert.Contains("VedSCEP handler threw NullReferenceException", msg);
+    }
+
+    [Fact]
+    public void Empty_error_body_stays_terse() {
+        Assert.Equal("HTTP 500", ScepHttpTransport.DescribeHttpError(500, string.Empty));
+        Assert.Equal("HTTP 500", ScepHttpTransport.DescribeHttpError(500, "   \r\n  "));
+    }
+
+    [Fact]
+    public void Long_error_body_is_truncated() {
+        string body;
+        string msg;
+
+        body = new string('x', 500) + "TAIL";
+        msg = ScepHttpTransport.DescribeHttpError(500, body);
+
+        Assert.True(msg.Length < 300, $"expected a truncated message, got {msg.Length} chars");
+        Assert.DoesNotContain("TAIL", msg);
+        Assert.Contains("…", msg);
+    }
+
+    [Fact]
+    public async Task Get_surfaces_the_server_error_body_on_non_2xx() {
+        StatusBodyHandler stub;
+        ScepHttpTransport transport;
+        ScepResult<byte[]> async_result;
+        ScepResult<byte[]> sync_result;
+
+        stub = new StatusBodyHandler { Status = HttpStatusCode.InternalServerError, Body = "vedscep exploded: see Windows event log" };
+        transport = new ScepHttpTransport(new HttpClient(stub), new Uri("https://host/vedscep/"), TimeSpan.FromSeconds(30));
+
+        async_result = await transport.GetAsync("GetCACaps", string.Empty);
+        Assert.False(async_result.IsOk);
+        Assert.Equal(ScepClientResult.NetworkError, async_result.Status);
+        Assert.Contains("500", async_result.Error);
+        Assert.Contains("vedscep exploded", async_result.Error);
+
+        sync_result = transport.Get("GetCACaps", string.Empty);
+        Assert.False(sync_result.IsOk);
+        Assert.Contains("vedscep exploded", sync_result.Error);
+    }
+
+    [Fact]
+    public void DescribeGet_returns_the_resolved_request_url() {
+        ScepHttpTransport transport;
+        string url;
+
+        transport = new ScepHttpTransport(new HttpClient(), new Uri("https://host/vedscep/"), TimeSpan.FromSeconds(30));
+        url = transport.DescribeGet("GetCACaps", string.Empty);
+
+        Assert.Contains("https://host/vedscep/", url);
+        Assert.Contains("operation=GetCACaps", url);
+    }
 }