fix: clear the open bug backlog (#473-#481, #483, #485-#487)

CHANGELOG.md

@@ -55,6 +55,27 @@ follow [Semantic Versioning](https://semver.org). Ongoing work is collected unde
   and obscure what really happened. Line breaks are now stripped from these values before they are
   logged.
 
+- **A REST API key is now confined to its own project on the project pages too.** The confinement
+  above was applied everywhere except the project endpoints themselves, which still judged a key
+  purely by who created it. Since keys are issued by an administrator, a key made for one project
+  could therefore list every project in the installation, open another project's details, and read
+  another project's **member email addresses**. Those three endpoints now apply the same rule as the
+  rest of the application: a key sees its own project and nothing else.
+
+- **A stored password can no longer end up in the log.** Whenever a user account was written to the
+  operator log — directly, or as part of something that mentions one, such as an API key's owner —
+  the scrambled form of that user's password went with it. It is scrambled rather than readable, so
+  nobody could have logged in with what was written, but it is exactly the material an attacker
+  wants for an offline guessing attack, and it should never leave the database. It is now masked
+  wherever an account is written out, the same way provider keys and mail passwords already were.
+
+- **The login session cookie no longer risks losing its "HTTPS only" marking.** The marking is
+  decided from which environment the application says it is running in, and that was worked out
+  differently in two places. Where an installation set both of the standard environment variables to
+  conflicting values, the two disagreed and the cookie could be sent without the marking on an
+  HTTPS installation, allowing it to travel over a plaintext connection. Both places now agree, and
+  environment-specific settings files are honoured everywhere.
+
 ### Changed
 
 - **Optimization proposals are now held to a stricter, correct standard of proof.** Two flaws made
@@ -70,6 +91,58 @@ follow [Semantic Versioning](https://semver.org). Ongoing work is collected unde
 
 ### Fixed
 
+- **A stalled provider no longer ties up the proxy indefinitely.** The five-minute limit on an
+  upstream call stopped applying as soon as the provider sent its response headers, so a provider
+  that answered and then went quiet mid-reply held the request, its connection and its worker open
+  until the calling application itself gave up — which, for a patient client, was never. Enough of
+  these and the proxy runs out of capacity. The limit now covers the whole reply, and a provider
+  that stalls is reported as a gateway timeout and recorded as one on the trace.
+
+- **A price-feed outage no longer makes refreshing a provider's models appear to hang.** Model
+  prices come from a public price list and a public exchange-rate feed, and a provider's models were
+  priced one at a time. If either feed was unreachable, every single model triggered its own full
+  retry, one after another — so a provider offering hundreds of models turned one refresh into
+  hundreds of doomed attempts against an already-failing address, and the refresh looked frozen. A
+  failed lookup is now remembered briefly for both feeds, so an outage costs one attempt instead of
+  hundreds, while a brief blip still recovers on the next refresh.
+
+- **Someone who belongs to several projects now gets a traces overview without naming one.** Asking
+  for the traces overview without specifying a project returned an empty overview for anyone who is
+  not an administrator and belongs to more than one project — the figures were only ever computed
+  for a single project at a time. The overview, and the evaluator sparklines beside it, are now
+  aggregated across every project the caller may see. The web app was never affected, because it
+  always names the current project.
+
+- **Closing a page with live updates no longer files an entry in the Error Log.** Navigating away
+  from — or simply closing — a view that streams live updates was recorded as an application error,
+  complete with an error reference that pointed at nothing an operator could act on. Ordinary
+  disconnects are now recognized as such and left out of the log; genuine faults are recorded exactly
+  as before.
+
+- **A test run that fails to hand off its results no longer reports itself finished twice.** If
+  something went wrong in the moment after a run group finished, the group announced its completion a
+  second time and was analysed for anomalies a second time — so a real anomaly could be flagged and
+  notified twice, while the log claimed the run had failed even though it is shown as completed. A
+  group now announces itself and queues its follow-up work exactly once, and a failure to queue one
+  of the two follow-up jobs no longer costs it the other.
+
+- **A test run that is cancelled just as it finishes no longer skips its follow-up analysis.** If a
+  cancellation arrived in the instant a run group was completing, the group was reported as
+  Completed but neither the optimizer nor anomaly detection ever looked at it — so that run silently
+  produced no improvement proposals and no anomaly flags, with nothing to show anything had been
+  missed. Once a group has finished, its follow-up work now always runs.
+
+- **A response that fails halfway through is no longer delivered as if it were complete.** When an
+  error struck after the server had already begun sending a reply, the connection was closed
+  tidily, so the caller received a well-formed but cut-off answer and had no way to tell it apart
+  from a genuinely short one — a truncated result could be consumed as real data. Such a failure now
+  breaks the connection, which callers and client libraries report as an error.
+
+- **An Azure OpenAI endpoint written in its fully-qualified form is now recognized.** A host name
+  ending in a dot — `resource.openai.azure.com.`, a legal way to write an absolute name — was not
+  detected as Azure, so model discovery used the wrong route and the Azure credential header was
+  left off, showing up as an empty model list and rejected calls with nothing pointing at the cause.
+
 - **A REST API key now sees its own project's data without having to name the project.** List
   endpoints take an optional project filter, and leaving it out was meant to mean "everything I am
   allowed to see". For anyone who is not an administrator it instead meant *nothing*: the request

Proxytrace.Api.Tests/AgentCallsControllerTests.cs

@@ -210,6 +210,63 @@ public async Task GetAll_AsNonAdminInSeveralProjectsWithoutFilter_ReturnsTheUnio
         result.Total.Should().Be(2);
     }
 
+    [TestMethod]
+    public async Task GetOverview_AsNonAdminInSeveralProjectsWithoutFilter_AggregatesTheUnion()
+    {
+        // #483: the overview's aggregates go through StatisticsFilter, which used to carry a single
+        // project id (partly applied in raw SQL). A caller who may read several projects and named
+        // none therefore got an empty overview instead of an aggregate over their own projects.
+        IServiceProvider services = GetServices();
+        var first = await SeedAgentInNewProjectAsync(services, "first");
+        var second = await SeedAgentInNewProjectAsync(services, "second");
+        var outsider = await SeedAgentInNewProjectAsync(services, "outsider");
+        await SeedCallWithToolsAsync(services, first, []);
+        await SeedCallWithToolsAsync(services, second, []);
+        await SeedCallWithToolsAsync(services, outsider, []);
+
+        var controller = ResolveController(services, ScopedGuard(first.Project.Id, second.Project.Id));
+        var overview = await controller.GetOverview(cancellationToken: CancellationToken);
+
+        overview.AgentBreakdown.Select(b => b.AgentId).Should().BeEquivalentTo([first.Id, second.Id]);
+        overview.Agents.Select(a => a.Id).Should().BeEquivalentTo([first.Id, second.Id]);
+        // The latency percentiles are the raw-SQL path in production; the third project's call must
+        // not be in the sample either.
+        overview.Latency.Sum(l => l.SampleCount).Should().Be(2);
+    }
+
+    [TestMethod]
+    public async Task GetOverview_AsNonAdminInOneProjectWithoutFilter_AggregatesThatProjectOnly()
+    {
+        // The single-project scope (the web UI, and every REST API key — confined to one project)
+        // keeps going through the filter's by-one-project branch, unchanged by #483.
+        IServiceProvider services = GetServices();
+        var mine = await SeedAgentInNewProjectAsync(services, "mine");
+        var theirs = await SeedAgentInNewProjectAsync(services, "theirs");
+        await SeedCallWithToolsAsync(services, mine, []);
+        await SeedCallWithToolsAsync(services, theirs, []);
+
+        var controller = ResolveController(services, ScopedGuard(mine.Project.Id));
+        var overview = await controller.GetOverview(cancellationToken: CancellationToken);
+
+        overview.AgentBreakdown.Should().ContainSingle().Which.AgentId.Should().Be(mine.Id);
+        overview.Agents.Select(a => a.Id).Should().Equal(mine.Id);
+    }
+
+    [TestMethod]
+    public async Task GetOverview_AsNonMember_ReturnsEmptyWithoutQuerying()
+    {
+        IServiceProvider services = GetServices();
+        var agent = await SeedAgentInNewProjectAsync(services, "theirs");
+        await SeedCallWithToolsAsync(services, agent, []);
+
+        var controller = ResolveController(services, DenyingGuard());
+        var overview = await controller.GetOverview(cancellationToken: CancellationToken);
+
+        overview.Agents.Should().BeEmpty();
+        overview.AgentBreakdown.Should().BeEmpty();
+        overview.Latency.Should().BeEmpty();
+    }
+
     /// <summary>
     /// An agent in a project of its own, so a test can tell two tenants' rows apart.
     /// </summary>

Proxytrace.Api.Tests/Config/HostEnvironmentNameTests.cs

@@ -0,0 +1,63 @@
+using AwesomeAssertions;
+using Proxytrace.Api.Configuration;
+
+namespace Proxytrace.Api.Tests.Config;
+
+/// <summary>
+/// Pins the environment-name resolution the container module shares with the host. The precedence
+/// is the whole point: <c>WebApplicationBuilder</c> lets <c>DOTNET_ENVIRONMENT</c> win over
+/// <c>ASPNETCORE_ENVIRONMENT</c>, and reading them the other way round defaulted the session
+/// cookie's <c>Secure</c> attribute to <c>false</c> on an HTTPS install.
+/// </summary>
+[TestClass]
+public sealed class HostEnvironmentNameTests
+{
+    private static Func<string, string?> Environment(string? dotnet = null, string? aspNetCore = null) =>
+        name => name switch
+        {
+            "DOTNET_ENVIRONMENT" => dotnet,
+            "ASPNETCORE_ENVIRONMENT" => aspNetCore,
+            _ => null,
+        };
+
+    [TestMethod]
+    public void Resolve_WithNeitherVariableSet_IsProduction() =>
+        HostEnvironmentName.Resolve(Environment()).Should().Be("Production");
+
+    [TestMethod]
+    public void Resolve_WithOnlyAspNetCoreSet_UsesIt() =>
+        HostEnvironmentName.Resolve(Environment(aspNetCore: "Staging")).Should().Be("Staging");
+
+    [TestMethod]
+    public void Resolve_WithOnlyDotnetSet_UsesIt() =>
+        HostEnvironmentName.Resolve(Environment(dotnet: "Staging")).Should().Be("Staging");
+
+    [TestMethod]
+    public void Resolve_WhenBothSetAndDisagree_PrefersDotnetLikeTheHost()
+    {
+        HostEnvironmentName.Resolve(Environment(dotnet: "Production", aspNetCore: "Development"))
+            .Should().Be("Production");
+
+        HostEnvironmentName.Resolve(Environment(dotnet: "Development", aspNetCore: "Production"))
+            .Should().Be("Development");
+    }
+
+    [TestMethod]
+    public void Resolve_WithBlankValues_TreatsThemAsUnset()
+    {
+        HostEnvironmentName.Resolve(Environment(dotnet: "", aspNetCore: "Development"))
+            .Should().Be("Development");
+
+        HostEnvironmentName.Resolve(Environment(dotnet: "   ", aspNetCore: "  "))
+            .Should().Be("Production");
+    }
+
+    [TestMethod]
+    public void IsDevelopment_MatchesCaseInsensitively()
+    {
+        HostEnvironmentName.IsDevelopment("development").Should().BeTrue();
+        HostEnvironmentName.IsDevelopment("Development").Should().BeTrue();
+        HostEnvironmentName.IsDevelopment("Production").Should().BeFalse();
+        HostEnvironmentName.IsDevelopment("Staging").Should().BeFalse();
+    }
+}

Proxytrace.Api.Tests/Middleware/ExceptionHandlingMiddlewareTests.cs

@@ -3,7 +3,9 @@
 using AwesomeAssertions;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
 using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using NSubstitute;
 using Proxytrace.Api.Middleware;
@@ -18,7 +20,10 @@ namespace Proxytrace.Api.Tests.Middleware;
 [TestClass]
 public sealed class ExceptionHandlingMiddlewareTests
 {
-    private static ExceptionHandlingMiddleware Create(RequestDelegate next, bool isDevelopment = false)
+    private static ExceptionHandlingMiddleware Create(
+        RequestDelegate next,
+        bool isDevelopment = false,
+        ILogger<ExceptionHandlingMiddleware>? logger = null)
     {
         var env = Substitute.For<IWebHostEnvironment>();
         env.EnvironmentName.Returns(isDevelopment ? "Development" : "Production");
@@ -35,7 +40,7 @@ private static ExceptionHandlingMiddleware Create(RequestDelegate next, bool isD
         ];
 
         return new ExceptionHandlingMiddleware(
-            next, NullLogger<ExceptionHandlingMiddleware>.Instance, mappers, env);
+            next, logger ?? NullLogger<ExceptionHandlingMiddleware>.Instance, mappers, env);
     }
 
     private static async Task<(int Status, string Body)> InvokeAsync(ExceptionHandlingMiddleware middleware)
@@ -50,6 +55,32 @@ private static ExceptionHandlingMiddleware Create(RequestDelegate next, bool isD
         return (ctx.Response.StatusCode, await reader.ReadToEndAsync());
     }
 
+    /// <summary>
+    /// Runs the middleware against a context whose response has already started, and hands back the
+    /// lifetime feature so the test can observe whether the connection was reset, plus the recorded
+    /// log levels so it can observe whether the fault was captured into the Error Log.
+    /// </summary>
+    private static async Task<(IHttpRequestLifetimeFeature Lifetime, RecordingLogger Log)>
+        InvokeAfterResponseStartedAsync(Exception thrown, bool clientDisconnected = false)
+    {
+        var response = Substitute.For<IHttpResponseFeature>();
+        response.HasStarted.Returns(true);
+        response.Headers.Returns(new HeaderDictionary());
+
+        var lifetime = Substitute.For<IHttpRequestLifetimeFeature>();
+        lifetime.RequestAborted.Returns(
+            clientDisconnected ? new CancellationToken(canceled: true) : CancellationToken.None);
+
+        var ctx = new DefaultHttpContext();
+        ctx.Features.Set(response);
+        ctx.Features.Set(lifetime);
+        ctx.Request.Path = "/api/agent-calls/stream";
+
+        var log = new RecordingLogger();
+        await Create(_ => throw thrown, logger: log).InvokeAsync(ctx);
+        return (lifetime, log);
+    }
+
     private static JsonElement Error(string body)
     {
         using var doc = JsonDocument.Parse(body);
@@ -225,6 +256,93 @@ await FluentActions
             .Should().ThrowAsync<OperationCanceledException>();
     }
 
+    [TestMethod]
+    public async Task InvokeAsync_ExceptionAfterResponseStarted_AbortsTheConnection()
+    {
+        // Returning normally would let the framework close the response cleanly, so the client would
+        // read a truncated body as a complete one. The reset is the only remaining failure signal.
+        var (lifetime, _) = await InvokeAfterResponseStartedAsync(new InvalidOperationException("boom"));
+
+        lifetime.Received(1).Abort();
+    }
+
+    [TestMethod]
+    public async Task InvokeAsync_ExceptionAfterResponseStarted_CapturesAnError()
+    {
+        // A genuine mid-stream fault on a live connection keeps its Error-level capture, so the
+        // Error Log row (and the errorId that deep-links to it) still exists for an operator.
+        var (_, log) = await InvokeAfterResponseStartedAsync(new InvalidOperationException("boom"));
+
+        log.Levels.Should().Contain(LogLevel.Error);
+    }
+
+    [TestMethod]
+    public async Task InvokeAsync_ExceptionAfterResponseStarted_DoesNotRethrow()
+    {
+        await FluentActions
+            .Invoking(() => InvokeAfterResponseStartedAsync(new InvalidOperationException("boom")))
+            .Should().NotThrowAsync();
+    }
+
+    [TestMethod]
+    public async Task InvokeAsync_ExceptionAfterClientDisconnected_DoesNotAbort()
+    {
+        // The connection is already gone — resetting it again would only add noise.
+        var (lifetime, _) = await InvokeAfterResponseStartedAsync(
+            new IOException("connection reset"), clientDisconnected: true);
+
+        lifetime.DidNotReceive().Abort();
+    }
+
+    [TestMethod]
+    public async Task InvokeAsync_ExceptionAfterClientDisconnected_DoesNotCaptureAnError()
+    {
+        // Every Error/Critical entry becomes an ApplicationError row, so an Error-level log here
+        // would put one Error Log entry in front of operators per closed browser tab — for a
+        // routine disconnect nobody can act on.
+        var (_, log) = await InvokeAfterResponseStartedAsync(
+            new IOException("connection reset"), clientDisconnected: true);
+
+        log.Levels.Should().NotContain(LogLevel.Error);
+    }
+
+    [TestMethod]
+    public async Task InvokeAsync_ExceptionAfterClientDisconnected_LogsOnceAtDebug()
+    {
+        var (_, log) = await InvokeAfterResponseStartedAsync(
+            new IOException("connection reset"), clientDisconnected: true);
+
+        log.Levels.Should().ContainSingle().Which.Should().Be(LogLevel.Debug);
+    }
+
+    /// <summary>
+    /// Records the level of every entry the middleware writes. Only Error/Critical entries are
+    /// picked up by the error-log capture pipeline, so the recorded levels tell a test whether a
+    /// fault was captured as an <c>ApplicationError</c> row.
+    /// </summary>
+    private sealed class RecordingLogger : ILogger<ExceptionHandlingMiddleware>
+    {
+        public List<LogLevel> Levels { get; } = [];
+
+        public IDisposable BeginScope<TState>(TState state) where TState : notnull => NullScope.Instance;
+
+        public bool IsEnabled(LogLevel logLevel) => true;
+
+        public void Log<TState>(
+            LogLevel logLevel, EventId eventId, TState state, Exception? exception,
+            Func<TState, Exception?, string> formatter)
+            => Levels.Add(logLevel);
+
+        private sealed class NullScope : IDisposable
+        {
+            public static readonly NullScope Instance = new();
+
+            public void Dispose()
+            {
+            }
+        }
+    }
+
     private sealed class FakeDbException : DbException
     {
         private readonly string sqlState;