Various Bugfixes - #484
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if (!group.Status.IsTerminal()) | ||
| // Same check-then-act as the cancel path: a user cancelling while the group is | ||
| // failing must not collide with this transition or its broadcast. | ||
| using (IDisposable sync = await asyncLock.LockAsync(group.Id, CancellationToken.None)) |
| group = await group.ReloadAsync(cancellationToken); | ||
| group = await group.SetCompleted(cancellationToken); | ||
| broadcaster.PublishGroupComplete(GroupRunCompleteEvent.Create(group)); | ||
| using (IDisposable sync = await asyncLock.LockAsync(group.Id, cancellationToken)) |
| { | ||
| var agent = await agentRepository.FindAsync(aid, cancellationToken); | ||
| return agent is not null && accessible.Contains(agent.Project.Id); | ||
| if (agent is null || !scope.Admits(agent.Project.Id)) |
| { | ||
| var agent = await agents.FindAsync(aid, cancellationToken); | ||
| return agent is not null && accessible.Contains(agent.Project.Id); | ||
| if (agent is null || !scope.Admits(agent.Project.Id)) |
|
|
||
| var byKey = await providers.FindByApiKeyAsync("sk-legacy-789", CancellationToken); | ||
|
|
||
| byKey.Should().NotBeNull(); |
|
|
||
| // The row is still recognised as un-backfilled and is hashed, so the key authenticates again. | ||
| var found = await keys.FindByKeyAsync(keyPlaintext, CancellationToken); | ||
| found.Should().NotBeNull(); |
| var latest = await repo.GetLatestByEvaluatorAsync(mine.Id, CancellationToken); | ||
|
|
||
| recent.Should().ContainSingle("this evaluator has exactly one result, however much else ran since"); | ||
| latest.Should().NotBeNull(); |
The new per-IP `auth-login` limiter (30/min) counts the whole e2e suite against a single partition: every spec authenticates its own ProxytraceApiClient, so a run drives ~80 real logins from one source address in ~3.4 minutes. The tail of the run 429'd — 25 failures, all `login failed: 429`. Raise the budget for the e2e stack only, via the operator override seam AuthRateLimiterConfigurator already exposes. The shipped default in appsettings.json is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DusPcXLc8APo1tUWKDHjmW
CodeQL flagged two `cs/log-forging` sites (alerts #38, #39). The request line cannot carry a raw newline, but a percent-encoded one survives URL decoding: `/x%0D%0AINFO:%20admin%20logged%20in` reaches `Request.Path` as two lines, and a flat-file or console sink renders the second as a log entry of its own. Add `ToSingleLogLine()` in Proxytrace.Common.Text and apply it at both sites — the "response already started" warning in ExceptionHandlingMiddleware and the missing-Passthrough-scope warning in OpenAiProxyController. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DusPcXLc8APo1tUWKDHjmW
Sweeps the seven pre-existing `cs/log-forging` alerts (#15–#21) with the `ToSingleLogLine()` helper added for #38/#39: the upstream-failure warnings in OpenAiProxyController and TraceyChatController, the Not-implemented sibling in ExceptionHandlingMiddleware, the two TestSupportController message logs, and both password-reset fallback warnings in PasswordResetService. These sites are on master rather than new to this branch, so they were not blocking the merge — but they are the same defect and are now shipped code, hence the CHANGELOG entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DusPcXLc8APo1tUWKDHjmW
The SARIF dataflow for the surviving cs/log-forging alert (#40) shows the tainted value on that line is `issued.ResetLink.Link`, not `user.Email`: it originates at AuthController.BuildResetUrl, where the configured frontend origin falls back to `Request.Scheme`/`Request.Host`, and is carried through PasswordResetLink into the emergency-log branch. Sanitize the link too. The email keeps its sanitizer as defence in depth (it is user-supplied at signup/invite); the redacted branch logs only a token hash, which is why CodeQL flags one branch and not the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DusPcXLc8APo1tUWKDHjmW
JabbaKadabra
added a commit
that referenced
this pull request
Jul 28, 2026
Fixes every bug open on the tracker, plus the three found while doing so. Security / access control - #474 ProjectsController delegated to IProjectAccessGuard. Its inline CanAccessAsync never read ApiKeyAuthenticationHandler.ProjectIdItemKey, so a REST key minted for project A could read project B's detail and its members' email addresses whenever the key's owner (usually an admin) was a member. - #473 User.PrintMembers masks PasswordHash. ExternalSubject stays visible: the existing five redact credentials, not identifiers. - #479 HostEnvironmentName resolves the environment as the host does (DOTNET_ENVIRONMENT ahead of ASPNETCORE_ENVIRONMENT) and the module layers in appsettings.{Environment}.json. The reversed order made a Production host compute Development and drop Secure from the session cookie. Proxy - #475 ResponseHeadersRead is kept, but ProxyBufferedResponseAsync re-arms the bound at its copy loop from client.Timeout, so a stalled upstream is a 504 instead of a request held open until the client gives up. - #480 The SSE splitter treats a lone CR as a terminator, carrying the CR/LF seam across a chunk boundary so CRLF is never counted twice. Aggregates and scoping - #483 StatisticsFilter gained ProjectIds, applied in both the EF path and the raw SQL one (= ANY(@projectIDS), a single uuid[] parameter, never interpolated). The single-project path is preserved, so the web UI and every REST key keep their indexed equality predicate and its plan. EvaluatorsController's sparklines had the identical constraint and got the identical treatment. Runner and middleware - #476 The success-path optimizer/anomaly enqueues take CancellationToken.None, matching the failure path: a cancel landing as the group completed skipped both jobs silently. - #486 The generic catch only broadcasts and enqueues for a group it actually transitioned. The two enqueues also absorb their own failures, so losing one no longer costs the group the other. - #477 A fault after Response.HasStarted aborts the connection. Returning signalled success, framing a truncated body as complete. - #485 A client that hung up mid-stream is classified before the error capture, so a closed tab no longer persists an ApplicationError nobody can act on. Pricing - #478/#487 The LiteLLM catalog and the Frankfurter FX feed each arm a 30s negative cache. Priced per model, an outage previously cost one full fetch per discovered model, serialized behind the gate. Misc - #481 IsAzure trims one trailing DNS root dot before the exact suffix match, keeping the domain-boundary guarantee (my-azure.com.example.net still fails). #482 needed no change: PR #484 already added ResolveListScopeAsync and moved all eight list endpoints onto it. Verified: dotnet build Proxytrace.sln clean (24 projects, 0 warnings); scoped test runs green (Domain 398, Storage 815, Proxy 96, Infrastructure 73, Application 523, Api 543); manual VitePress build clean. Not verified: the two perf metrics added for #483 carry uncalibrated placeholder budgets. perf/run.sh --size 1000000 is still owed to set them from a real p95. Claude-Session: https://claude.ai/code/session_01RTyMUobDLv1aEAqHwtUtQg Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
JabbaKadabra
added a commit
that referenced
this pull request
Jul 30, 2026
Resolves the conflicts between the cost-control feature and the bug-backlog and security work merged to master (#484, #488, 86f72d8). - AuditAction / NotificationKind: both sides appended members. Master's already-merged values keep their numbering; the cost actions shift to 72-76 and CostBudget moves after TraceQuotaReached. Neither shifted value has shipped, so no stored row changes meaning. - ResolvedApiKey: each side added a member for the same reason (attributing a proxied call to the Proxytrace-issued key). The record now carries both ApiKeyId (per-key spend and budget blocking) and Scopes (the pass-through capability check), both null on the upstream-key path. - OpenAiProxyController: keeps the ResolvedApiKey-shaped capture signatures and takes master's additions - the body-download timeout (#475) and capturedStatus, so a stalled upstream is still traced as a 504. - IApiKeyRepository / ApiKeyRepository: additive, GetByProjectAsync alongside GetKeyNamesByOwnerAsync. - CHANGELOG: merged section by section under [Unreleased]. Master's two separate ### Fixed headings are folded into one; entry order and text are unchanged. Verified: dotnet build (0 warnings), dotnet test Proxytrace.sln (2930 passed, 0 failed), frontend build/lint/test (1140 passed), manual docs:build, and `ef migrations has-pending-model-changes` (none). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9NmoPS88gGTwTKAuUcbNb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.