From f0702a15a008a1880147cb8878d3cae186bbce11 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Fri, 9 Oct 2026 18:55:06 +0800 Subject: [PATCH] Harden webhook ingestion against outsiders Anyone who could reach the anonymous webhook endpoints could start runs, replay deliveries and bury the refusal log: - A fork PR from someone outside a public repository started a run like a member's, with nothing in the payload to tell them apart, and every close/reopen started another run. - GitHub signs the body only, so a captured delivery posted again under a fresh (or no) X-GitHub-Delivery started new runs for as long as the secret was valid. - Every forged-signature post wrote its own audit row with unbounded headers, so fifty of them pushed every genuine refusal off the panel. - The whole body was read before the hook was looked up, with no size or rate limit on the webhook and callback endpoints. PR trigger payloads now carry authorAssociation, isFork and headRepositoryFullName. GitLab sends no standing, so the author's project access level is looked up once per delivery (Developer and above is a member). trigger.pr.opened and trigger.pr.updated gain an 'authors' filter (any | members). Members means whoever made the code what it is: the author, and for new commits on a fork head whoever pushed them. A member can open a PR from an outsider's fork branch, and the outsider's later pushes must not run on the member's standing. GitLab's pusher is looked up like the author; GitHub has no lookup here, so another user's push to a fork head is not admitted. Existing activations set no 'authors'. They admit only members when the delivery says the repository is public or internal, anyone when it says private, and, when it does not say, only members for a fork PR. Internal counts with public because every user of a GitLab instance or GitHub enterprise can fork it and open a PR. So existing activations on public and internal repositories stop running outsiders' PRs; private ones behave as before. A run holds off the next one from the same activation for the same PR at the same head commit for 60 seconds. A close/reopen loop re-sends a head that already ran; a push moves the head, so a member's follow-up push always gets its own run. A delivery that already started its run is skipped as a duplicate before admission, so posting it again (an operator's Redeliver, or a captured body with its own id) cannot restart that window. GitHub deliveries without X-GitHub-Delivery are refused with 400, and a signed body is claimed per hook for 7 days, so the same body under another id starts nothing while a redelivery, which keeps its id, still reaches dispatch. Refusals anyone can cause collapse to one row per hook and reason per day, with headers capped. Webhook bodies are capped at 25 MiB and callbacks at 1 MiB, and the hook is resolved before its body is read. Each remote address gets 10,000 ingress requests a minute across every hook and callback, then 2,000 per (hook or token, address). The address budget is checked first because the per-target key is a value the sender chooses: a sender changing it on every request was never limited and left a limiter behind each time. The callback route admits only the engine's 32-character token shape, so a key can no longer be kilobytes long. Claims live in a new webhook_claim table (migration 0244), taken inside the delivery's transaction so a delivery that rolls back releases them. Expired claims are taken over in place and deleted by a recurring sweep every five minutes that skips rows a delivery holds; purging them inside a delivery held their locks until it committed, and two deliveries could deadlock. Activations are visited in id order so concurrent deliveries take their debounce claims in the same order. --- .../Controllers/WebhooksController.cs | 51 +- .../WorkflowCallbacksController.cs | 22 +- .../WebhookIngressRateLimitExtension.cs | 76 +++ .../CodeSpace.Api/Http/BoundedRequestBody.cs | 46 ++ backend/src/CodeSpace.Api/Startup.cs | 1 + ...PurgeExpiredWebhookClaimsCommandHandler.cs | 20 + .../Webhooks/WebhookExistsQueryHandler.cs | 15 + .../WebhookClaimPurgeRecurringJob.cs | 24 + .../DbUpFiles/0244_webhook_claim.sql | 35 ++ .../IRepositoryMemberStandingCapability.cs | 18 + .../Capabilities/IWebhookDeliveryIdentity.cs | 17 + .../Providers/GitHub/Events/GitHubDelivery.cs | 2 +- .../GitHubPullRequestEventSubscription.cs | 76 ++- .../GitHub/GitHubRepositoryProvider.cs | 4 +- .../GitLabMergeRequestEventSubscription.cs | 44 +- ...GitLabRepositoryProvider.MemberStanding.cs | 56 ++ .../WebhookDeliveryUnidentifiedException.cs | 17 + .../Services/Webhooks/IWebhookClaimStore.cs | 19 + .../Services/Webhooks/IWebhookLookup.cs | 9 + .../Webhooks/RejectedDeliveryHeaders.cs | 39 ++ .../Services/Webhooks/WebhookClaimStore.cs | 64 +++ .../Webhooks/WebhookIngestionService.Audit.cs | 86 ++- .../Webhooks/WebhookIngestionService.cs | 32 +- .../Services/Webhooks/WebhookLookup.cs | 18 + .../Nodes/Builtin/PrTriggerSchemas.cs | 57 +- .../Nodes/Builtin/TriggerPrOpenedNode.cs | 7 +- .../Nodes/Builtin/TriggerPrUpdatedNode.cs | 7 +- .../PullRequestAuthorRefusedException.cs | 20 + .../PullRequestTriggerDebouncedException.cs | 21 + .../Admission/IPullRequestTriggerAdmission.cs | 21 + .../Admission/PullRequestTriggerAdmission.cs | 180 +++++++ .../Admission/PullRequestTriggerAuthors.cs | 59 ++ .../RunSources/Matchers/PrOpenedMatcher.cs | 5 +- .../RunSources/Matchers/PrUpdatedMatcher.cs | 5 +- .../Matchers/PullRequestOriginPayload.cs | 13 + .../RunSources/RunSourceDispatcher.cs | 70 ++- .../PurgeExpiredWebhookClaimsCommand.cs | 20 + .../WorkflowRunRequestRejectionReasons.cs | 28 + .../Enums/PullRequestAuthorAssociation.cs | 21 + .../PullRequest/PullRequestOpenedEvent.cs | 8 +- .../Events/PullRequest/PullRequestOrigin.cs | 52 ++ .../PullRequestSynchronizedEvent.cs | 7 +- .../Queries/Webhooks/WebhookExistsQuery.cs | 17 + .../WebhookIngressSourceLimitE2ETests.cs | 91 ++++ .../WebhookOutsiderHardeningE2ETests.cs | 509 ++++++++++++++++++ .../Webhooks/WebhookTenancyE2ETests.cs | 9 +- .../ConnectionWebhookIngestionTests.cs | 47 ++ .../Webhooks/ReceiveWebhookFlowTests.cs | 2 + .../Webhooks/WebhookClaimStoreTests.cs | 181 +++++++ .../Webhooks/WebhookIngressContractTests.cs | 88 +++ .../WebhookIngressRefusalFlowTests.cs | 188 +++++++ .../PullRequestTriggerAdmissionFlowTests.cs | 412 ++++++++++++++ .../SweepCommandTransactionFlowTests.cs | 1 + .../RequestAuthorizationInventoryTests.cs | 2 + .../Jobs/PeriodicJanitorDispatchTests.cs | 52 +- .../GitHub/GitHubPullRequestOriginTests.cs | 129 +++++ .../GitLab/GitLabMemberStandingTests.cs | 37 ++ .../GitLab/GitLabMergeRequestOriginTests.cs | 100 ++++ .../Webhooks/IngressRefusalFailureTests.cs | 32 ++ .../Webhooks/RejectedDeliveryHeadersTests.cs | 63 +++ .../Webhooks/WebhookIngestionKeysTests.cs | 62 +++ .../Workflows/NodeManifestContractTests.cs | 33 +- .../PullRequestTriggerAuthorsTests.cs | 112 ++++ .../RunSourceDispatcherTenancyTests.cs | 90 +++- .../Workflows/TriggerMatcherTests.cs | 38 ++ ...WorkflowRunRequestRejectionReasonsTests.cs | 4 + frontend/src/api/types.ts | 1 + .../RepositoryWebhooksPanel.test.tsx | 50 ++ frontend/src/lib/webhookState.ts | 28 + 69 files changed, 3685 insertions(+), 85 deletions(-) create mode 100644 backend/src/CodeSpace.Api/Extensions/WebhookIngressRateLimitExtension.cs create mode 100644 backend/src/CodeSpace.Api/Http/BoundedRequestBody.cs create mode 100644 backend/src/CodeSpace.Core/Handlers/CommandHandlers/Webhooks/PurgeExpiredWebhookClaimsCommandHandler.cs create mode 100644 backend/src/CodeSpace.Core/Handlers/QueryHandlers/Webhooks/WebhookExistsQueryHandler.cs create mode 100644 backend/src/CodeSpace.Core/Jobs/RecurringJobs/WebhookClaimPurgeRecurringJob.cs create mode 100644 backend/src/CodeSpace.Core/Persistence/DbUpFiles/0244_webhook_claim.sql create mode 100644 backend/src/CodeSpace.Core/Services/Providers/Capabilities/IRepositoryMemberStandingCapability.cs create mode 100644 backend/src/CodeSpace.Core/Services/Providers/Capabilities/IWebhookDeliveryIdentity.cs create mode 100644 backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.MemberStanding.cs create mode 100644 backend/src/CodeSpace.Core/Services/Webhooks/Exceptions/WebhookDeliveryUnidentifiedException.cs create mode 100644 backend/src/CodeSpace.Core/Services/Webhooks/IWebhookClaimStore.cs create mode 100644 backend/src/CodeSpace.Core/Services/Webhooks/IWebhookLookup.cs create mode 100644 backend/src/CodeSpace.Core/Services/Webhooks/RejectedDeliveryHeaders.cs create mode 100644 backend/src/CodeSpace.Core/Services/Webhooks/WebhookClaimStore.cs create mode 100644 backend/src/CodeSpace.Core/Services/Webhooks/WebhookLookup.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestAuthorRefusedException.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestTriggerDebouncedException.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/IPullRequestTriggerAdmission.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAdmission.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAuthors.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PullRequestOriginPayload.cs create mode 100644 backend/src/CodeSpace.Messages/Commands/Webhooks/PurgeExpiredWebhookClaimsCommand.cs create mode 100644 backend/src/CodeSpace.Messages/Enums/PullRequestAuthorAssociation.cs create mode 100644 backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOrigin.cs create mode 100644 backend/src/CodeSpace.Messages/Queries/Webhooks/WebhookExistsQuery.cs create mode 100644 backend/tests/CodeSpace.E2ETests/Webhooks/WebhookIngressSourceLimitE2ETests.cs create mode 100644 backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookClaimStoreTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressContractTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressRefusalFlowTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/PullRequestTriggerAdmissionFlowTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubPullRequestOriginTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMemberStandingTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMergeRequestOriginTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Webhooks/IngressRefusalFailureTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Webhooks/RejectedDeliveryHeadersTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Webhooks/WebhookIngestionKeysTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/PullRequestTriggerAuthorsTests.cs diff --git a/backend/src/CodeSpace.Api/Controllers/WebhooksController.cs b/backend/src/CodeSpace.Api/Controllers/WebhooksController.cs index 1f637762c..570e09292 100644 --- a/backend/src/CodeSpace.Api/Controllers/WebhooksController.cs +++ b/backend/src/CodeSpace.Api/Controllers/WebhooksController.cs @@ -1,29 +1,49 @@ +using CodeSpace.Api.Extensions; +using CodeSpace.Api.Http; +using CodeSpace.Core.Services.Webhooks.Exceptions; using CodeSpace.Messages.Commands.Webhooks; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Queries.Webhooks; using MediatR; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.RateLimiting; namespace CodeSpace.Api.Controllers; +/// +/// Provider deliveries. Anonymous — the per-hook signature is the proof — so everything here is bounded before it is +/// authenticated: the rate limiter answers a flood before the action runs, an id naming no hook is answered before the +/// body is read, and a body is read only up to . +/// [ApiController] [Route("api/webhooks")] [AllowAnonymous] +[EnableRateLimiting(WebhookIngressRateLimitExtension.PolicyName)] public class WebhooksController : ControllerBase { + /// GitHub caps a webhook payload at 25 MB and drops a larger event rather than send it; GitLab truncates its lists well below that. Nothing a provider sends is bigger. + public const long MaxDeliveryBodyBytes = 25L * 1024 * 1024; + private readonly IMediator _mediator; public WebhooksController(IMediator mediator) { _mediator = mediator; } [HttpPost("{webhookId:guid}")] + [RequestSizeLimit(MaxDeliveryBodyBytes)] public async Task Receive(Guid webhookId, CancellationToken cancellationToken) { + if (!await HookExistsAsync(ProviderWebhookScope.Repository, webhookId, cancellationToken).ConfigureAwait(false)) return NotFound(); + var delivery = await ReadDeliveryAsync(cancellationToken).ConfigureAwait(false); + if (delivery == null) return StatusCode(StatusCodes.Status413PayloadTooLarge); + return await DispatchAsync(new ReceiveWebhookCommand { WebhookId = webhookId, - Body = delivery.Body, - Headers = delivery.Headers + Body = delivery.Value.Body, + Headers = delivery.Value.Headers }, cancellationToken).ConfigureAwait(false); } @@ -34,22 +54,32 @@ public async Task Receive(Guid webhookId, CancellationToken cance /// the wrong table and answered 404. /// [HttpPost("connection/{connectionWebhookId:guid}")] + [RequestSizeLimit(MaxDeliveryBodyBytes)] public async Task ReceiveConnection(Guid connectionWebhookId, CancellationToken cancellationToken) { + if (!await HookExistsAsync(ProviderWebhookScope.Connection, connectionWebhookId, cancellationToken).ConfigureAwait(false)) return NotFound(); + var delivery = await ReadDeliveryAsync(cancellationToken).ConfigureAwait(false); + if (delivery == null) return StatusCode(StatusCodes.Status413PayloadTooLarge); + return await DispatchAsync(new ReceiveConnectionWebhookCommand { ConnectionWebhookId = connectionWebhookId, - Body = delivery.Body, - Headers = delivery.Headers + Body = delivery.Value.Body, + Headers = delivery.Value.Headers }, cancellationToken).ConfigureAwait(false); } - private async Task<(string Body, Dictionary Headers)> ReadDeliveryAsync(CancellationToken cancellationToken) + private async Task HookExistsAsync(ProviderWebhookScope scope, Guid webhookId, CancellationToken cancellationToken) => + await _mediator.Send(new WebhookExistsQuery { WebhookId = webhookId, Scope = scope }, cancellationToken).ConfigureAwait(false); + + /// Null when the body is larger than . + private async Task<(string Body, Dictionary Headers)?> ReadDeliveryAsync(CancellationToken cancellationToken) { - using var reader = new StreamReader(Request.Body); - var body = await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false); + var body = await Request.ReadBoundedBodyAsync(MaxDeliveryBodyBytes, cancellationToken).ConfigureAwait(false); + + if (body == null) return null; return (body, Request.Headers.ToDictionary(h => h.Key, h => h.Value.ToString())); } @@ -58,7 +88,8 @@ public async Task ReceiveConnection(Guid connectionWebhookId, Can /// The one status mapping, shared by both routes so a group delivery and a project delivery /// cannot answer differently for the same failure. 200 for anything the service handled — a /// provider reads 5xx as "retry" and GitLab disables a hook that keeps failing, which would take - /// out every repository the hook covers. + /// out every repository the hook covers. 400 for a signed delivery missing the id its provider + /// always sends — malformed for that provider, and never something the provider itself sends. /// private async Task DispatchAsync(IRequest command, CancellationToken cancellationToken) { @@ -72,6 +103,10 @@ private async Task DispatchAsync(IRequest command, Cancella { return Unauthorized(); } + catch (WebhookDeliveryUnidentifiedException) + { + return BadRequest(); + } catch (InvalidOperationException) { return NotFound(); diff --git a/backend/src/CodeSpace.Api/Controllers/WorkflowCallbacksController.cs b/backend/src/CodeSpace.Api/Controllers/WorkflowCallbacksController.cs index 959cb7fe0..754cb5a6a 100644 --- a/backend/src/CodeSpace.Api/Controllers/WorkflowCallbacksController.cs +++ b/backend/src/CodeSpace.Api/Controllers/WorkflowCallbacksController.cs @@ -1,7 +1,10 @@ +using CodeSpace.Api.Extensions; +using CodeSpace.Api.Http; using CodeSpace.Messages.Commands.Workflows; using MediatR; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.RateLimiting; namespace CodeSpace.Api.Controllers; @@ -9,22 +12,33 @@ namespace CodeSpace.Api.Controllers; /// Unauthenticated callback surface: an external system resumes a run parked on /// flow.wait_callback by POSTing to the tokened URL the run-detail UI shows. The token is /// the bearer secret (high-entropy, single-use while the wait is pending) — there is no team / -/// user context, mirroring . +/// user context, mirroring , and bounded the same way: rate-limited +/// per address and then per (token, address), and read only up to . /// [ApiController] [Route("api/workflows/callbacks")] [AllowAnonymous] +[EnableRateLimiting(WebhookIngressRateLimitExtension.PolicyName)] public class WorkflowCallbacksController : ControllerBase { + /// The body becomes the waiting node's output, stored in a jsonb row and read by every later step. A resume signal has no business being larger. + public const long MaxCallbackBodyBytes = 1024 * 1024; + private readonly IMediator _mediator; public WorkflowCallbacksController(IMediator mediator) { _mediator = mediator; } - [HttpPost("{token}")] + /// + /// The route admits only the shape the engine mints — Guid.NewGuid().ToString("N"), 32 characters. A token no + /// wait can hold is answered 404 by routing, before the limiter keys a partition on it or anything looks it up. + /// + [HttpPost("{token:length(32)}")] + [RequestSizeLimit(MaxCallbackBodyBytes)] public async Task Resume([FromRoute] string token, CancellationToken cancellationToken) { - using var reader = new StreamReader(Request.Body); - var body = await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false); + var body = await Request.ReadBoundedBodyAsync(MaxCallbackBodyBytes, cancellationToken).ConfigureAwait(false); + + if (body == null) return StatusCode(StatusCodes.Status413PayloadTooLarge); var resumed = await _mediator.Send(new ResumeWorkflowCallbackCommand { Token = token, Body = body }, cancellationToken).ConfigureAwait(false); diff --git a/backend/src/CodeSpace.Api/Extensions/WebhookIngressRateLimitExtension.cs b/backend/src/CodeSpace.Api/Extensions/WebhookIngressRateLimitExtension.cs new file mode 100644 index 000000000..362ce3825 --- /dev/null +++ b/backend/src/CodeSpace.Api/Extensions/WebhookIngressRateLimitExtension.cs @@ -0,0 +1,76 @@ +using System.Threading.RateLimiting; +using Microsoft.AspNetCore.RateLimiting; + +namespace CodeSpace.Api.Extensions; + +/// +/// A ceiling on what one source can make the anonymous ingress endpoints do — provider webhooks and workflow callbacks. +/// Both are reached before anything is authenticated, and both used to answer every request without a limit. +/// +/// Two limits, in order. First per remote address, across every ingress endpoint: the app's +/// GlobalLimiter, which ASP.NET consults before any endpoint policy and which passes every other endpoint +/// through untouched. Then per (target, address), where the target is the hook id or callback token in the URL. The +/// order is the point. The target is a value the sender picks, and each distinct one gets a limiter of its own that +/// outlives its window, so a sender changing the target on every request was never limited and left a limiter behind +/// each time. Checked first, the address budget runs out, and it bounds how many per-target limiters one sender can +/// create. The routes bound each target's length: a hook id is a GUID, a callback token the 32 characters the engine +/// mints. +/// +/// Sized for a provider, not a person. A group / organization hook subscribes to every event — pipeline and job +/// events included — so a busy group sends hundreds a minute to one hook, and a self-managed instance sends every hook's +/// traffic from one address. A provider reads a refusal as a failed delivery: GitLab disables a hook that keeps failing. +/// So the per-hook ceiling sits well above one busy hook, and the per-address one above several of them at once. +/// +/// The address is the connection's own; nothing here reads forwarded headers. Behind a proxy that hides the +/// client address, every sender shares one address and so one budget. +/// +public static class WebhookIngressRateLimitExtension +{ + public const string PolicyName = "webhook-ingress"; + + /// Per (target, address) per window. + public const int PermitsPerWindow = 2000; + + /// Per address per window, across every ingress target — five hooks at their full budget. + public const int SourcePermitsPerWindow = 10_000; + + public static readonly TimeSpan Window = TimeSpan.FromMinutes(1); + + /// The route values that name what a request is addressed to, in the order the ingress routes declare them. + private static readonly string[] TargetRouteValues = { "webhookId", "connectionWebhookId", "token" }; + + /// The one partition every non-ingress request shares in the global limiter — no limit. Never an address. + private const string NotIngress = "not-ingress"; + + public static void AddWebhookIngressRateLimit(this IServiceCollection services) + { + services.AddRateLimiter(options => + { + options.RejectionStatusCode = StatusCodes.Status429TooManyRequests; + options.GlobalLimiter = PartitionedRateLimiter.Create(SourcePartition); + options.AddPolicy(PolicyName, context => FixedWindow(PartitionKey(context), PermitsPerWindow)); + }); + } + + /// The address's budget for an ingress request; no limit for anything else the app serves. + private static RateLimitPartition SourcePartition(HttpContext context) => + IsIngress(context) ? FixedWindow(SourcePartitionKey(context), SourcePermitsPerWindow) : RateLimitPartition.GetNoLimiter(NotIngress); + + /// An endpoint that opted into the ingress policy — the same marker the per-target limit is applied by. + private static bool IsIngress(HttpContext context) => + context.GetEndpoint()?.Metadata.GetMetadata()?.PolicyName == PolicyName; + + private static RateLimitPartition FixedWindow(string key, int permits) => + RateLimitPartition.GetFixedWindowLimiter(key, _ => new FixedWindowRateLimiterOptions { PermitLimit = permits, Window = Window, QueueLimit = 0 }); + + /// Who sent the request, and nothing the sender chose. + internal static string SourcePartitionKey(HttpContext context) => context.Connection.RemoteIpAddress?.ToString() ?? "unknown"; + + /// "{target}|{address}". A request with no target route value shares one partition per address. + internal static string PartitionKey(HttpContext context) + { + var target = TargetRouteValues.Select(name => context.GetRouteValue(name)?.ToString()).FirstOrDefault(value => !string.IsNullOrEmpty(value)) ?? "-"; + + return $"{target}|{SourcePartitionKey(context)}"; + } +} diff --git a/backend/src/CodeSpace.Api/Http/BoundedRequestBody.cs b/backend/src/CodeSpace.Api/Http/BoundedRequestBody.cs new file mode 100644 index 000000000..ded3e9c16 --- /dev/null +++ b/backend/src/CodeSpace.Api/Http/BoundedRequestBody.cs @@ -0,0 +1,46 @@ +namespace CodeSpace.Api.Http; + +/// +/// Reads an anonymous endpoint's body only up to a limit. [RequestSizeLimit] makes Kestrel stop at the same +/// limit, but it does so by THROWING from the read, which the global exception filter would turn into a 500, and a host +/// without the size feature does not stop at all. This answers "too large" the same way under either. +/// +public static class BoundedRequestBody +{ + /// The body as text, or null when it is longer than . + public static async Task ReadBoundedBodyAsync(this HttpRequest request, long maxBytes, CancellationToken cancellationToken) + { + if (request.ContentLength > maxBytes) return null; + + try + { + using var buffer = await ReadUpToAsync(request.Body, maxBytes + 1, cancellationToken).ConfigureAwait(false); + if (buffer.Length > maxBytes) return null; + + using var reader = new StreamReader(buffer); + return await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false); + } + catch (BadHttpRequestException ex) when (ex.StatusCode == StatusCodes.Status413PayloadTooLarge) + { + return null; + } + } + + /// At most bytes of the body, rewound for reading. + private static async Task ReadUpToAsync(Stream body, long limit, CancellationToken cancellationToken) + { + var buffer = new MemoryStream(); + var chunk = new byte[81920]; + + while (buffer.Length < limit) + { + var read = await body.ReadAsync(chunk.AsMemory(0, (int)Math.Min(chunk.Length, limit - buffer.Length)), cancellationToken).ConfigureAwait(false); + if (read == 0) break; + + buffer.Write(chunk, 0, read); + } + + buffer.Position = 0; + return buffer; + } +} diff --git a/backend/src/CodeSpace.Api/Startup.cs b/backend/src/CodeSpace.Api/Startup.cs index a1c3feecc..c41aac430 100644 --- a/backend/src/CodeSpace.Api/Startup.cs +++ b/backend/src/CodeSpace.Api/Startup.cs @@ -95,6 +95,7 @@ public void ConfigureServices(IServiceCollection services) services.Configure(o => o.ShutdownTimeout = ShutdownSettings.ResolveDrainTimeout()); services.AddAnonymousRateLimit(); + services.AddWebhookIngressRateLimit(); services.AddCustomAuthentication(Configuration, Environment); } diff --git a/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Webhooks/PurgeExpiredWebhookClaimsCommandHandler.cs b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Webhooks/PurgeExpiredWebhookClaimsCommandHandler.cs new file mode 100644 index 000000000..d61a54a72 --- /dev/null +++ b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Webhooks/PurgeExpiredWebhookClaimsCommandHandler.cs @@ -0,0 +1,20 @@ +using CodeSpace.Core.Services.Webhooks; +using CodeSpace.Messages.Commands.Webhooks; +using MediatR; + +namespace CodeSpace.Core.Handlers.CommandHandlers.Webhooks; + +/// Rule 16 — thin handler. The expiry predicate and the batched delete live in . +public sealed class PurgeExpiredWebhookClaimsCommandHandler : IRequestHandler +{ + private readonly IWebhookClaimStore _claims; + + public PurgeExpiredWebhookClaimsCommandHandler(IWebhookClaimStore claims) { _claims = claims; } + + public async Task Handle(PurgeExpiredWebhookClaimsCommand request, CancellationToken cancellationToken) + { + var deleted = await _claims.PurgeExpiredAsync(cancellationToken).ConfigureAwait(false); + + return new PurgeExpiredWebhookClaimsResponse { Deleted = deleted }; + } +} diff --git a/backend/src/CodeSpace.Core/Handlers/QueryHandlers/Webhooks/WebhookExistsQueryHandler.cs b/backend/src/CodeSpace.Core/Handlers/QueryHandlers/Webhooks/WebhookExistsQueryHandler.cs new file mode 100644 index 000000000..20a105171 --- /dev/null +++ b/backend/src/CodeSpace.Core/Handlers/QueryHandlers/Webhooks/WebhookExistsQueryHandler.cs @@ -0,0 +1,15 @@ +using CodeSpace.Core.Services.Webhooks; +using CodeSpace.Messages.Queries.Webhooks; +using MediatR; + +namespace CodeSpace.Core.Handlers.QueryHandlers.Webhooks; + +public sealed class WebhookExistsQueryHandler : IRequestHandler +{ + private readonly IWebhookLookup _lookup; + + public WebhookExistsQueryHandler(IWebhookLookup lookup) { _lookup = lookup; } + + public async Task Handle(WebhookExistsQuery request, CancellationToken cancellationToken) => + await _lookup.ExistsAsync(request.Scope, request.WebhookId, cancellationToken).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Jobs/RecurringJobs/WebhookClaimPurgeRecurringJob.cs b/backend/src/CodeSpace.Core/Jobs/RecurringJobs/WebhookClaimPurgeRecurringJob.cs new file mode 100644 index 000000000..9880866a3 --- /dev/null +++ b/backend/src/CodeSpace.Core/Jobs/RecurringJobs/WebhookClaimPurgeRecurringJob.cs @@ -0,0 +1,24 @@ +using CodeSpace.Messages.Commands.Webhooks; +using MediatR; + +namespace CodeSpace.Core.Jobs.RecurringJobs; + +/// +/// Every 5 minutes, dispatches to clear expired webhook_claim rows. +/// Thin Mediator dispatcher (Rule 14) — the delete lives in . +/// +/// Replaces a bounded purge each claim used to run inside its own delivery's transaction. That held the purged +/// rows' locks until the delivery committed, and two deliveries that each purged a batch and then claimed a key in the +/// other's batch deadlocked — one of them failing with a 5xx the provider does not redeliver on its own. +/// +public sealed class WebhookClaimPurgeRecurringJob : IRecurringJob +{ + private readonly IMediator _mediator; + + public WebhookClaimPurgeRecurringJob(IMediator mediator) { _mediator = mediator; } + + public string JobId => nameof(WebhookClaimPurgeRecurringJob); + public string CronExpression => "*/5 * * * *"; // every 5 minutes + + public async Task Execute() => await _mediator.Send(new PurgeExpiredWebhookClaimsCommand()).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0244_webhook_claim.sql b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0244_webhook_claim.sql new file mode 100644 index 000000000..18dc87305 --- /dev/null +++ b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0244_webhook_claim.sql @@ -0,0 +1,35 @@ +-- 0244_webhook_claim.sql +-- +-- A key held for a window, so that "this already happened moments ago" is settled by the database instead of by a read +-- that two deliveries landing together can both pass. Two webhook-ingress decisions ride it: +-- +-- body:{hook}:{sha256} A signed GitHub body this hook accepted, held by its X-GitHub-Delivery id. GitHub's +-- signature covers the body only, so a captured body stays valid for as long as the +-- secret does; posting it again under a fresh delivery id is a replay. The same id +-- (a provider redelivery) re-claims it and is deduplicated further on, per activation. +-- pr-debounce:{activation}:{repository}:{number}:{head} +-- The pull request head commit that last started a run from this activation. A +-- close/reopen loop re-sends a head that already ran, so it starts one run inside the +-- window, not one per reopen; a push moves the head, so new commits always run. +-- +-- A claim is taken inside the delivery's own transaction, so a delivery that rolls back releases it and the provider's +-- retry is not refused, and it touches no row but its own key. An expired claim is taken over in place by the next claim +-- on its key; the recurring WebhookClaimPurgeRecurringJob deletes the rest, outside any delivery. (A delivery that +-- purged expired rows itself held their locks until it committed, and two such deliveries could deadlock.) +-- +-- holder is TEXT, not a bounded VARCHAR: it is the delivery id from an unsigned header, and a value too long for a +-- column would fail the delivery with a 500 instead of being refused or deduplicated. +-- +-- New table, no backfill. Idempotent (IF NOT EXISTS). An older pod never reads or writes it. + +CREATE TABLE IF NOT EXISTS webhook_claim ( + claim_key TEXT PRIMARY KEY, + holder TEXT NOT NULL, + claimed_at TIMESTAMPTZ NOT NULL, + expires_at TIMESTAMPTZ NOT NULL +); + +CREATE INDEX IF NOT EXISTS ix_webhook_claim_expires_at ON webhook_claim (expires_at); + +COMMENT ON TABLE webhook_claim IS + 'A key held until expires_at by one holder: a replayed webhook body (body:) or a debounced pull request head (pr-debounce:). Taken in the delivery transaction; expired rows are taken over in place by a claim on the same key and deleted by a recurring sweep.'; diff --git a/backend/src/CodeSpace.Core/Services/Providers/Capabilities/IRepositoryMemberStandingCapability.cs b/backend/src/CodeSpace.Core/Services/Providers/Capabilities/IRepositoryMemberStandingCapability.cs new file mode 100644 index 000000000..4328d30ff --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Providers/Capabilities/IRepositoryMemberStandingCapability.cs @@ -0,0 +1,18 @@ +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Providers.Capabilities; + +/// +/// How a provider user — named by their provider id — stands with a repository. For a webhook that names a pull +/// request's author but not their standing: GitLab's merge request hook carries the author id and no access level, so a +/// trigger that admits only members has to ask. +/// +/// Rule 7 (ISP): a sibling of , which reports the CREDENTIAL's own role. A +/// provider whose payload already says (GitHub's author_association) does not implement this. +/// +public interface IRepositoryMemberStandingCapability : IProviderCapability +{ + /// Member or None from the provider's answer, Unknown when it answered with an error. A provider that cannot be reached throws; either way a caller treats the author as not a member. + Task GetMemberStandingAsync(ProviderContext context, RemoteRepository repository, string userExternalId, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Providers/Capabilities/IWebhookDeliveryIdentity.cs b/backend/src/CodeSpace.Core/Services/Providers/Capabilities/IWebhookDeliveryIdentity.cs new file mode 100644 index 000000000..3bc209afc --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Providers/Capabilities/IWebhookDeliveryIdentity.cs @@ -0,0 +1,17 @@ +namespace CodeSpace.Core.Services.Providers.Capabilities; + +/// +/// A provider whose webhook signature covers the BODY and which stamps every delivery with an id in a header (GitHub's +/// HMAC and X-GitHub-Delivery). A body signed that way stays valid for as long as the secret does, so anyone who +/// captured one can post it again; ingestion therefore refuses such a delivery without its id, and refuses a body the hook +/// already accepted under a different id. +/// +/// Not implemented by a provider that authenticates with a static token header (GitLab's X-Gitlab-Token): +/// whoever holds that token can sign any body at all, so a replay check would stop nothing the token does not already +/// allow — and an older GitLab sends no delivery id to require. +/// +public interface IWebhookDeliveryIdentity : IProviderCapability +{ + /// The header every authentic delivery carries its id in. + string DeliveryIdHeader { get; } +} diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubDelivery.cs b/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubDelivery.cs index 827864f73..5656c7126 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubDelivery.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubDelivery.cs @@ -2,7 +2,7 @@ namespace CodeSpace.Core.Services.Providers.GitHub.Events; internal static class GitHubDelivery { - private const string Header = "X-GitHub-Delivery"; + internal const string Header = "X-GitHub-Delivery"; /// Reads the GitHub delivery id from the webhook header; falls back to a random N-format Guid so downstream events always have a stable ProviderEventId. public static string IdFromHeadersOrFallback(IReadOnlyDictionary headers) diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubPullRequestEventSubscription.cs b/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubPullRequestEventSubscription.cs index 196ade1a1..3e11390a1 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubPullRequestEventSubscription.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitHub/Events/GitHubPullRequestEventSubscription.cs @@ -20,10 +20,10 @@ public sealed class GitHubPullRequestEventSubscription : IProviderEventSubscript return action switch { - "opened" => BuildOpened(repositoryId, deliveryId, now, pr), + "opened" => BuildOpened(repositoryId, deliveryId, now, pr, root), // Reopening re-enters the "needs review / CI" state. GitHub Actions bundles `reopened` // into its default pull_request trigger set alongside `opened`, so we fire the same event. - "reopened" => BuildOpened(repositoryId, deliveryId, now, pr), + "reopened" => BuildOpened(repositoryId, deliveryId, now, pr, root), "synchronize" => BuildSynchronized(repositoryId, deliveryId, now, pr, root), "closed" => pr.GetProperty("merged").GetBoolean() ? BuildMerged(repositoryId, deliveryId, now, pr, root) @@ -32,7 +32,7 @@ public sealed class GitHubPullRequestEventSubscription : IProviderEventSubscript }; } - private static PullRequestOpenedEvent BuildOpened(Guid repositoryId, string deliveryId, DateTimeOffset now, JsonElement pr) + private static PullRequestOpenedEvent BuildOpened(Guid repositoryId, string deliveryId, DateTimeOffset now, JsonElement pr, JsonElement root) { var user = pr.GetProperty("user"); @@ -50,8 +50,10 @@ private static PullRequestOpenedEvent BuildOpened(Guid repositoryId, string deli AuthorExternalId = user.GetProperty("id").GetRawText(), AuthorName = user.GetProperty("login").GetString()!, WebUrl = pr.GetProperty("html_url").GetString()!, + HeadSha = TryReadSide(pr, "head", out var head) ? ReadString(head, "sha") : null, Labels = ExtractLabels(pr), - IsDraft = ReadIsDraft(pr) + IsDraft = ReadIsDraft(pr), + Origin = ReadOrigin(pr, root) }; } @@ -65,9 +67,73 @@ private static PullRequestOpenedEvent BuildOpened(Guid repositoryId, string deli PreviousHeadSha = root.GetProperty("before").GetString()!, NewHeadSha = root.GetProperty("after").GetString()!, Labels = ExtractLabels(pr), - IsDraft = ReadIsDraft(pr) + IsDraft = ReadIsDraft(pr), + Origin = ReadOrigin(pr, root) with { PusherExternalId = ReadSenderId(root) } }; + /// On a synchronize, sender is whoever pushed the commits — not necessarily the PR's author. Null when the payload omits it. + private static string? ReadSenderId(JsonElement root) => + root.TryGetProperty("sender", out var sender) && sender.ValueKind == JsonValueKind.Object && sender.TryGetProperty("id", out var id) ? id.GetRawText() : null; + + /// + /// Who wrote the PR and where its head lives. Every field is optional on the wire: a payload that omits one reads as + /// "not known", never as a member or a same-repository PR — except a head repository GitHub sends as null, + /// which it does only for a fork that was since deleted. + /// + private static PullRequestOrigin ReadOrigin(JsonElement pr, JsonElement root) => new() + { + AuthorExternalId = pr.TryGetProperty("user", out var user) && user.ValueKind == JsonValueKind.Object && user.TryGetProperty("id", out var id) ? id.GetRawText() : null, + AuthorAssociation = MapAuthorAssociation(ReadString(pr, "author_association")), + IsFork = ReadIsFork(pr), + HeadRepositoryFullName = TryReadSide(pr, "head", out var head) && head.TryGetProperty("repo", out var repo) && repo.ValueKind == JsonValueKind.Object ? ReadString(repo, "full_name") : null, + RepositoryVisibility = root.TryGetProperty("repository", out var repository) && repository.ValueKind == JsonValueKind.Object ? ReadVisibility(repository) : null + }; + + /// + /// GitHub's author_association onto the normalised standing. OWNER, MEMBER and COLLABORATOR hold a role on the + /// repository or its organization; the contributor values have had changes merged but hold none; anything else is + /// unknown rather than guessed. + /// + internal static PullRequestAuthorAssociation MapAuthorAssociation(string? raw) => raw switch + { + "OWNER" or "MEMBER" or "COLLABORATOR" => PullRequestAuthorAssociation.Member, + "CONTRIBUTOR" or "FIRST_TIME_CONTRIBUTOR" or "FIRST_TIMER" => PullRequestAuthorAssociation.Contributor, + "NONE" or "MANNEQUIN" => PullRequestAuthorAssociation.None, + _ => PullRequestAuthorAssociation.Unknown + }; + + /// + /// Whether the head lives in another repository than the base. Not head.repo.fork: that says the head + /// REPOSITORY is a fork of something, which is true of every branch in an organization's own fork of upstream. + /// + private static bool ReadIsFork(JsonElement pr) + { + if (!TryReadSide(pr, "head", out var head) || !head.TryGetProperty("repo", out var headRepo)) return false; + if (headRepo.ValueKind == JsonValueKind.Null) return true; + if (!TryReadSide(pr, "base", out var @base) || !@base.TryGetProperty("repo", out var baseRepo)) return false; + if (headRepo.ValueKind != JsonValueKind.Object || baseRepo.ValueKind != JsonValueKind.Object) return false; + if (!headRepo.TryGetProperty("id", out var headId) || !baseRepo.TryGetProperty("id", out var baseId)) return false; + + return headId.GetRawText() != baseId.GetRawText(); + } + + /// repository.visibility when GitHub sends it (it distinguishes internal), else the older repository.private flag. + private static RepositoryVisibility? ReadVisibility(JsonElement repository) => ReadString(repository, "visibility") switch + { + "public" => RepositoryVisibility.Public, + "internal" => RepositoryVisibility.Internal, + "private" => RepositoryVisibility.Private, + _ => repository.TryGetProperty("private", out var isPrivate) && isPrivate.ValueKind is JsonValueKind.True or JsonValueKind.False + ? (isPrivate.GetBoolean() ? RepositoryVisibility.Private : RepositoryVisibility.Public) + : null + }; + + private static bool TryReadSide(JsonElement pr, string side, out JsonElement value) => + pr.TryGetProperty(side, out value) && value.ValueKind == JsonValueKind.Object; + + private static string? ReadString(JsonElement element, string name) => + element.TryGetProperty(name, out var value) && value.ValueKind == JsonValueKind.String ? value.GetString() : null; + /// /// GitHub sets pull_request.draft = true while a PR is a draft. Absent / non-boolean → /// false (treat as ready), so a provider that omits the field never makes a PR look like a draft. diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs index 21a19eb26..d7cf0c17c 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs @@ -22,7 +22,7 @@ namespace CodeSpace.Core.Services.Providers.GitHub; -public sealed partial class GitHubRepositoryProvider : IRepositoryCatalogCapability, IPullRequestCatalogCapability, IPullRequestCommentCapability, IPullRequestReviewCapability, IPullRequestWriteCapability, IIssueCatalogCapability, IIssueWriteCapability, IReleaseCatalogCapability, IRepositoryAccessCapability, IRepositorySourceCapability, IRepositoryInsightsCapability, IRepositoryHistoryCapability, IRepositoryMarkdownRenderCapability, ICredentialProbeCapability, IWebhookRegistrationCapability, IWebhookSignatureVerifier, IWebhookEventNormalizer +public sealed partial class GitHubRepositoryProvider : IRepositoryCatalogCapability, IPullRequestCatalogCapability, IPullRequestCommentCapability, IPullRequestReviewCapability, IPullRequestWriteCapability, IIssueCatalogCapability, IIssueWriteCapability, IReleaseCatalogCapability, IRepositoryAccessCapability, IRepositorySourceCapability, IRepositoryInsightsCapability, IRepositoryHistoryCapability, IRepositoryMarkdownRenderCapability, ICredentialProbeCapability, IWebhookRegistrationCapability, IWebhookSignatureVerifier, IWebhookEventNormalizer, IWebhookDeliveryIdentity { private readonly IProviderAuthResolver _authResolver; private readonly IExternalCallResilience _resilience; @@ -985,6 +985,8 @@ await _resilience.ExecuteAsync(context.Instance, nameof(DeleteWebhookAsync), public NormalizedEvent? Normalize(Guid repositoryId, string body, IReadOnlyDictionary headers) => _eventNormalizer.Normalize(repositoryId, body, headers); + public string DeliveryIdHeader => Events.GitHubDelivery.Header; + // ── Repository source (Code browser): branches, one tree level, single file ── public async Task> ListBranchesAsync(ProviderContext context, RemoteRepository repository, CancellationToken cancellationToken) diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitLab/Events/GitLabMergeRequestEventSubscription.cs b/backend/src/CodeSpace.Core/Services/Providers/GitLab/Events/GitLabMergeRequestEventSubscription.cs index 3ae3c2dd9..e946d5b49 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitLab/Events/GitLabMergeRequestEventSubscription.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitLab/Events/GitLabMergeRequestEventSubscription.cs @@ -50,11 +50,50 @@ private static PullRequestOpenedEvent BuildOpened(Guid repositoryId, string deli AuthorExternalId = user.GetProperty("id").GetRawText(), AuthorName = user.GetProperty("username").GetString()!, WebUrl = attrs.GetProperty("url").GetString()!, + HeadSha = ReadLastCommitId(attrs), Labels = ExtractLabels(root), - IsDraft = ReadIsDraft(attrs) + IsDraft = ReadIsDraft(attrs), + Origin = ReadOrigin(attrs, root) }; } + /// object_attributes.last_commit.id — the MR's head commit. Null when the payload omits it. + private static string? ReadLastCommitId(JsonElement attrs) => + attrs.TryGetProperty("last_commit", out var commit) && commit.ValueKind == JsonValueKind.Object && commit.TryGetProperty("id", out var id) && id.ValueKind == JsonValueKind.String ? id.GetString() : null; + + /// On an update that moved the head, user is whoever pushed — the one MR delivery whose actor made the code what it is. Null when the payload omits it. + private static string? ReadPusherId(JsonElement root) => + root.TryGetProperty("user", out var user) && user.ValueKind == JsonValueKind.Object && user.TryGetProperty("id", out var id) && id.ValueKind == JsonValueKind.Number ? id.GetRawText() : null; + + /// + /// Who wrote the MR and where its source branch lives. The author is object_attributes.author_id, not + /// user — that is whoever caused THIS delivery, and a maintainer reopening an outsider's MR must not lend it + /// their standing. GitLab's payload carries no standing at all, so the association stays Unknown here and is looked + /// up at dispatch, when a trigger needs it. + /// + private static PullRequestOrigin ReadOrigin(JsonElement attrs, JsonElement root) => new() + { + AuthorExternalId = attrs.TryGetProperty("author_id", out var authorId) && authorId.ValueKind == JsonValueKind.Number ? authorId.GetRawText() : null, + IsFork = ReadIsFork(attrs), + HeadRepositoryFullName = attrs.TryGetProperty("source", out var source) && source.ValueKind == JsonValueKind.Object && source.TryGetProperty("path_with_namespace", out var path) && path.ValueKind == JsonValueKind.String ? path.GetString() : null, + RepositoryVisibility = root.TryGetProperty("project", out var project) && project.ValueKind == JsonValueKind.Object && project.TryGetProperty("visibility_level", out var level) && level.ValueKind == JsonValueKind.Number ? MapVisibilityLevel(level.GetRawText()) : null + }; + + /// A fork MR is one whose source project is not its target project. Absent ids say nothing, so they are not a fork. + private static bool ReadIsFork(JsonElement attrs) => + attrs.TryGetProperty("source_project_id", out var source) && source.ValueKind == JsonValueKind.Number + && attrs.TryGetProperty("target_project_id", out var target) && target.ValueKind == JsonValueKind.Number + && source.GetRawText() != target.GetRawText(); + + /// GitLab's visibility_level: 0 private, 10 internal, 20 public. Any other value is not guessed at. + private static RepositoryVisibility? MapVisibilityLevel(string level) => level switch + { + "0" => RepositoryVisibility.Private, + "10" => RepositoryVisibility.Internal, + "20" => RepositoryVisibility.Public, + _ => null + }; + /// /// GitLab fires action:"update" for ANY merge-request mutation — label / assignee / /// description / milestone edits AND code pushes alike. It includes @@ -84,7 +123,8 @@ private static PullRequestSynchronizedEvent BuildSynchronized(Guid repositoryId, PreviousHeadSha = oldRev, NewHeadSha = newRev, Labels = ExtractLabels(root), - IsDraft = ReadIsDraft(attrs) + IsDraft = ReadIsDraft(attrs), + Origin = ReadOrigin(attrs, root) with { PusherExternalId = ReadPusherId(root) } }; } diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.MemberStanding.cs b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.MemberStanding.cs new file mode 100644 index 000000000..00a8c0870 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.MemberStanding.cs @@ -0,0 +1,56 @@ +using System.Text.Json; +using CodeSpace.Core.Services.Providers.Capabilities; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Providers.GitLab; + +/// +/// A merge request author's standing on the project, for the trigger's authors filter. GitLab's hook names the author +/// and not their access level, so this asks GET /projects/:id/members/all/:user_id — "all" so a role inherited +/// from a parent group counts like a direct one. +/// +public sealed partial class GitLabRepositoryProvider : IRepositoryMemberStandingCapability +{ + /// Developer: the first role that can push to the project — GitLab's counterpart of a GitHub collaborator. + internal const int MemberAccessLevelFloor = 30; + + public async Task GetMemberStandingAsync(ProviderContext context, RemoteRepository repository, string userExternalId, CancellationToken cancellationToken) + { + var auth = await _authResolver.ResolveAsync(context, cancellationToken).ConfigureAwait(false); + var host = string.IsNullOrWhiteSpace(context.Instance.ApiUrl) ? context.Instance.BaseUrl : context.Instance.ApiUrl; + var url = $"{host.TrimEnd('/')}/api/v4/projects/{Uri.EscapeDataString(repository.ExternalId)}/members/all/{Uri.EscapeDataString(userExternalId)}"; + + using var request = new HttpRequestMessage(HttpMethod.Get, url); + request.Headers.TryAddWithoutValidation("Authorization", $"Bearer {auth.Token}"); + request.Headers.TryAddWithoutValidation("PRIVATE-TOKEN", auth.Token); + + using var response = await _countsHttpClient.SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); + + if ((int)response.StatusCode == 404) return PullRequestAuthorAssociation.None; + if (!response.IsSuccessStatusCode) return PullRequestAuthorAssociation.Unknown; + + var json = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); + + return MapMemberStanding(ParseMemberAccessLevel(json)); + } + + /// Developer and above is a member; Guest and Reporter can see the project but hold no say over its code. + internal static PullRequestAuthorAssociation MapMemberStanding(int? accessLevel) => + accessLevel >= MemberAccessLevelFloor ? PullRequestAuthorAssociation.Member : PullRequestAuthorAssociation.None; + + /// The member body's access_level, or null when it is missing, not a number, or the body is not JSON. + internal static int? ParseMemberAccessLevel(string json) + { + try + { + using var doc = JsonDocument.Parse(json); + + return doc.RootElement.ValueKind == JsonValueKind.Object && doc.RootElement.TryGetProperty("access_level", out var level) && level.ValueKind == JsonValueKind.Number ? level.GetInt32() : null; + } + catch (JsonException) + { + return null; + } + } +} diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/Exceptions/WebhookDeliveryUnidentifiedException.cs b/backend/src/CodeSpace.Core/Services/Webhooks/Exceptions/WebhookDeliveryUnidentifiedException.cs new file mode 100644 index 000000000..0cc9c3d2e --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Webhooks/Exceptions/WebhookDeliveryUnidentifiedException.cs @@ -0,0 +1,17 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Core.Services.Webhooks.Exceptions; + +/// +/// A correctly signed delivery from a provider that stamps every delivery with an id arrived without one. The request +/// is malformed for that provider — a real one never sends it — so it is answered 400, not as a signature failure. +/// +public sealed class WebhookDeliveryUnidentifiedException : Exception, IFailure +{ + public WebhookDeliveryUnidentifiedException(Guid webhookId, string header) : base($"Webhook {webhookId} delivery carries no {header}") { } + + /// Malformed for its provider: the same request sent again is refused again. + public FailureKind Kind => FailureKind.Invalid; + + public string Code => FailureCodes.InvalidRequest; +} diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/IWebhookClaimStore.cs b/backend/src/CodeSpace.Core/Services/Webhooks/IWebhookClaimStore.cs new file mode 100644 index 000000000..b55e5df30 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Webhooks/IWebhookClaimStore.cs @@ -0,0 +1,19 @@ +namespace CodeSpace.Core.Services.Webhooks; + +/// +/// A key held for a window by one holder — how webhook ingress says "this already happened moments ago" without a read +/// two concurrent deliveries could both pass. The primary key settles the race; the loser waits for the winner's +/// transaction and then sees its claim. +/// +public interface IWebhookClaimStore +{ + /// + /// True when was free, had expired, or is already held by — the last + /// so that a provider redelivering the same delivery is not mistaken for a new one. False when someone else holds it. + /// Joins the caller's transaction, so a delivery that rolls back releases its claim. + /// + Task TryClaimAsync(string key, string holder, TimeSpan holdFor, CancellationToken cancellationToken); + + /// Deletes every claim whose window has ended, without waiting on one a delivery is holding. Returns how many it deleted. Called by the recurring sweep, never from a delivery. + Task PurgeExpiredAsync(CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/IWebhookLookup.cs b/backend/src/CodeSpace.Core/Services/Webhooks/IWebhookLookup.cs new file mode 100644 index 000000000..32eaa7d6b --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Webhooks/IWebhookLookup.cs @@ -0,0 +1,9 @@ +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Webhooks; + +/// Whether a hook id names a hook — asked before a delivery's body is read, so a body posted to an id naming nothing never is. +public interface IWebhookLookup +{ + Task ExistsAsync(ProviderWebhookScope scope, Guid webhookId, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/RejectedDeliveryHeaders.cs b/backend/src/CodeSpace.Core/Services/Webhooks/RejectedDeliveryHeaders.cs new file mode 100644 index 000000000..ee5d0061f --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Webhooks/RejectedDeliveryHeaders.cs @@ -0,0 +1,39 @@ +using System.Text.Json; + +namespace CodeSpace.Core.Services.Webhooks; + +/// +/// A refused delivery's headers as its audit row keeps them: names always, values only for a few safe ones, and all of it +/// bounded. The row is written before anything is authenticated, so whoever posts chooses the header names and the +/// safe-listed values — sixty forged posts with a two-thousand-character header name once stored 135 KB. The provider's +/// own headers are kept first, so the cap never drops the ones an operator matches against the provider's delivery log. +/// +internal static class RejectedDeliveryHeaders +{ + public const int MaxHeaders = 40; + + public const int MaxNameLength = 64; + + public const int MaxValueLength = 256; + + /// The entry that counts what the cap left out. + public const string OmittedKey = "(omitted)"; + + private const string Redacted = "[REDACTED]"; + + private static readonly HashSet SafeNames = new(StringComparer.OrdinalIgnoreCase) { "Content-Type", "User-Agent", "X-GitHub-Event", "X-GitHub-Delivery", "X-Gitlab-Event", "X-Gitlab-Event-UUID" }; + + public static string Serialize(IReadOnlyDictionary headers) + { + var kept = new Dictionary(StringComparer.OrdinalIgnoreCase); + + foreach (var (name, value) in headers.OrderByDescending(h => SafeNames.Contains(h.Key)).Take(MaxHeaders)) + kept[Truncate(name, MaxNameLength)] = SafeNames.Contains(name) ? Truncate(value, MaxValueLength) : Redacted; + + if (headers.Count > MaxHeaders) kept[OmittedKey] = $"{headers.Count - MaxHeaders} more headers not kept"; + + return JsonSerializer.Serialize(kept); + } + + private static string Truncate(string text, int max) => text.Length <= max ? text : $"{text[..max]}…"; +} diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/WebhookClaimStore.cs b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookClaimStore.cs new file mode 100644 index 000000000..d9781346c --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookClaimStore.cs @@ -0,0 +1,64 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using Microsoft.EntityFrameworkCore; + +namespace CodeSpace.Core.Services.Webhooks; + +/// +/// The webhook_claim table (migration 0244). One upsert decides a claim, inside the delivery's transaction, and +/// touches no row but its own key. Expired rows are taken over in place by the next claim on the same key; everything +/// else expired is cleared by , from the recurring sweep, outside any delivery. +/// +public sealed class WebhookClaimStore : IWebhookClaimStore, IScopedDependency +{ + /// How many expired claims one sweep statement deletes. Each batch commits on its own, so a backlog never holds a long lock that a delivery's claim could queue behind. + internal const int PurgeBatch = 1000; + + private readonly CodeSpaceDbContext _db; + + public WebhookClaimStore(CodeSpaceDbContext db) { _db = db; } + + /// + /// Insert, or take over a row whose window ended or that this holder already holds. A row held by someone else inside + /// its window matches the conflict but not the WHERE, so nothing is written and zero rows come back. + /// + public async Task TryClaimAsync(string key, string holder, TimeSpan holdFor, CancellationToken cancellationToken) + { + var written = await _db.Database.ExecuteSqlInterpolatedAsync($""" + INSERT INTO webhook_claim (claim_key, holder, claimed_at, expires_at) + VALUES ({key}, {holder}, statement_timestamp(), statement_timestamp() + {holdFor}) + ON CONFLICT (claim_key) DO UPDATE + SET holder = EXCLUDED.holder, claimed_at = EXCLUDED.claimed_at, expires_at = EXCLUDED.expires_at + WHERE webhook_claim.expires_at <= EXCLUDED.claimed_at OR webhook_claim.holder = EXCLUDED.holder + """, cancellationToken).ConfigureAwait(false); + + return written == 1; + } + + /// Every expired claim, a batch at a time until a batch comes back short. Returns how many were deleted. + public async Task PurgeExpiredAsync(CancellationToken cancellationToken) + { + var total = 0; + int deleted; + + do + { + deleted = await DeleteExpiredBatchAsync(cancellationToken).ConfigureAwait(false); + total += deleted; + } + while (deleted == PurgeBatch); + + return total; + } + + /// + /// SKIP LOCKED: a row a delivery holds — an expired claim it is taking over, uncommitted — is stepped around, + /// never waited on. The sweep waiting on a delivery while that delivery waited on a row the sweep had deleted is a + /// deadlock; a sweep that never waits cannot be half of one. + /// + private async Task DeleteExpiredBatchAsync(CancellationToken cancellationToken) => + await _db.Database.ExecuteSqlInterpolatedAsync($""" + DELETE FROM webhook_claim + WHERE claim_key IN (SELECT claim_key FROM webhook_claim WHERE expires_at < statement_timestamp() ORDER BY expires_at LIMIT {PurgeBatch} FOR UPDATE SKIP LOCKED) + """, cancellationToken).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.Audit.cs b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.Audit.cs index 55df68d45..9ea4d2e2f 100644 --- a/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.Audit.cs +++ b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.Audit.cs @@ -22,6 +22,25 @@ public sealed partial class WebhookIngestionService /// How long one event type nothing acts on collapses into a single audit row. See . private static readonly TimeSpan UnmappedAuditWindow = TimeSpan.FromDays(1); + /// + /// How long one (hook, reason) refusal that anyone can cause collapses into a single row: an inactive or retired hook, + /// a bad signature, a missing delivery id. Whoever knows a hook's URL can post as often as they like before anything is + /// authenticated, and a row per post once pushed every genuine refusal out of the operator's newest-fifty list. One row + /// a day still says "this is happening", as the unbound and unmapped rows already do. + /// + public static readonly TimeSpan RefusalAuditWindow = TimeSpan.FromDays(1); + + /// + /// How long a hook remembers a signed body it accepted. Longer than GitHub keeps a delivery redeliverable, so a body + /// captured from a delivery log cannot simply wait it out; a captured body older than this is still valid for as long + /// as the secret is, which rotating the secret ends. + /// + public static readonly TimeSpan ReplayWindow = TimeSpan.FromDays(7); + + /// One key per (hook, reason, day) — see . The rejected: keyspace's other forms carry a delivery id or an activation, so this one cannot collide with them. + internal static string BuildRefusalWindowKey(Guid webhookId, string reason, DateTimeOffset now) => + $"refused:{webhookId:N}:{reason}:{now.UtcTicks / RefusalAuditWindow.Ticks}"; + private async Task EnsureActiveOrAuditAsync(bool active, IngestionSubject subject, IReadOnlyDictionary headers, CancellationToken cancellationToken) { if (active) return; @@ -37,6 +56,7 @@ await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext Detail = $"webhook {subject.WebhookId} is configured as inactive", SourceType = BuildSourceType(subject), ExternalEventId = null, // pre-classification — we never read the body for an inactive webhook + DedupKey = BuildRefusalWindowKey(subject.WebhookId, WorkflowRunRequestRejectionReasons.WebhookInactive, DateTimeOffset.UtcNow), RawHeadersRedactedJson = SerializeRedactedHeaders(headers), }, cancellationToken).ConfigureAwait(false); @@ -67,6 +87,7 @@ await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext Detail = $"webhook {subject.WebhookId} was retired ({status}) and no longer accepts deliveries", SourceType = BuildSourceType(subject), ExternalEventId = null, // pre-classification — a retired hook's body is never read + DedupKey = BuildRefusalWindowKey(subject.WebhookId, WorkflowRunRequestRejectionReasons.WebhookRetired, DateTimeOffset.UtcNow), RawHeadersRedactedJson = SerializeRedactedHeaders(headers), }, cancellationToken).ConfigureAwait(false); @@ -92,6 +113,7 @@ await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext Detail = $"webhook {subject.WebhookId} belongs to repository {repository.Id}, which was removed, and no longer accepts deliveries", SourceType = BuildSourceType(subject), ExternalEventId = null, // pre-classification — a retired hook's body is never read + DedupKey = BuildRefusalWindowKey(subject.WebhookId, WorkflowRunRequestRejectionReasons.WebhookRetired, DateTimeOffset.UtcNow), RawHeadersRedactedJson = SerializeRedactedHeaders(headers), }, cancellationToken).ConfigureAwait(false); @@ -116,6 +138,7 @@ await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext Detail = $"signature did not validate for webhook {subject.WebhookId}", SourceType = BuildSourceType(subject), ExternalEventId = null, // body is untrusted — we don't extract delivery id pre-verification + DedupKey = BuildRefusalWindowKey(subject.WebhookId, WorkflowRunRequestRejectionReasons.SignatureInvalid, DateTimeOffset.UtcNow), RawHeadersRedactedJson = SerializeRedactedHeaders(headers), VerificationResultJson = JsonSerializer.Serialize(new { validated = false, verifier_class = verifier.GetType().Name }), }, cancellationToken).ConfigureAwait(false); @@ -123,6 +146,54 @@ await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext throw new UnauthorizedAccessException("Webhook signature verification failed"); } + /// + /// A provider that stamps every delivery with an id () sent a correctly signed + /// one without it. A real provider never does; a tool replaying a captured body does, because without an id every + /// post of that body would read as a new delivery. Answered 400 and recorded once per hook per day. + /// + private async Task EnsureDeliveryIdentifiedOrAuditAsync(IngestionSubject subject, IReadOnlyDictionary headers, CancellationToken cancellationToken) + { + if (!_registry.TryGet(subject.Provider, out var identity) || identity == null) return; + if (Providers.WebhookHeaderLookup.TryFind(headers, identity.DeliveryIdHeader, out var deliveryId) && !string.IsNullOrWhiteSpace(deliveryId)) return; + + _logger.LogWarning("Webhook {WebhookId} delivery was signed but carried no {Header}", subject.WebhookId, identity.DeliveryIdHeader); + + await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext + { + TeamId = subject.TeamId, + RepositoryId = subject.RepositoryId, + Reason = WorkflowRunRequestRejectionReasons.DeliveryIdMissing, + Detail = $"a signed delivery to webhook {subject.WebhookId} carried no {identity.DeliveryIdHeader}; {subject.Provider} sends one with every delivery", + SourceType = BuildSourceType(subject), + ExternalEventId = null, + DedupKey = BuildRefusalWindowKey(subject.WebhookId, WorkflowRunRequestRejectionReasons.DeliveryIdMissing, DateTimeOffset.UtcNow), + RawHeadersRedactedJson = SerializeRedactedHeaders(headers), + }, cancellationToken).ConfigureAwait(false); + + throw new Exceptions.WebhookDeliveryUnidentifiedException(subject.WebhookId, identity.DeliveryIdHeader); + } + + /// + /// This hook already accepted the identical signed body under another delivery id. Answered 200 like any delivery + /// handled, and recorded once per replayed body however often it is posted — the claim key already names the body. + /// + private async Task AuditReplayedDeliveryAsync(IngestionSubject subject, string claimKey, string deliveryId, IReadOnlyDictionary headers, CancellationToken cancellationToken) + { + _logger.LogWarning("Webhook {WebhookId} delivery {DeliveryId} repeats a body this hook already accepted under another delivery id; not started again", subject.WebhookId, deliveryId); + + await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext + { + TeamId = subject.TeamId, + RepositoryId = subject.RepositoryId, + Reason = WorkflowRunRequestRejectionReasons.DeliveryReplayed, + Detail = $"delivery {deliveryId} to webhook {subject.WebhookId} carries a signed body this hook already accepted under another delivery id in the last {ReplayWindow.TotalDays:0} days; nothing was started", + SourceType = BuildSourceType(subject), + ExternalEventId = deliveryId, + DedupKey = $"replayed:{claimKey}", + RawHeadersRedactedJson = SerializeRedactedHeaders(headers), + }, cancellationToken).ConfigureAwait(false); + } + /// /// Signature passed but the body couldn't be parsed into the expected shape (non-JSON, or /// missing / mistyped fields a normalizer requires — a provider API change, a truncated @@ -294,19 +365,8 @@ private static string BuildUnboundDedupKey(Guid connectionWebhookId, WebhookRepo private static string BuildSourceType(IngestionSubject subject) => $"{WorkflowRunSourceTypes.ProviderPrefix}{subject.Provider.ToString().ToLowerInvariant()}"; - /// - /// Serialise the request headers with secret/auth values stripped. The audit row stores - /// header NAMES (operators want to see "Authorization was present" without leaking the - /// token); add header values only for explicitly safe ones (Content-Type, User-Agent). - /// - private static string SerializeRedactedHeaders(IReadOnlyDictionary headers) - { - var safeNames = new HashSet(StringComparer.OrdinalIgnoreCase) { "Content-Type", "User-Agent", "X-GitHub-Event", "X-GitHub-Delivery", "X-Gitlab-Event", "X-Gitlab-Event-UUID" }; - var redacted = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var (name, value) in headers) - redacted[name] = safeNames.Contains(name) ? value : "[REDACTED]"; - return JsonSerializer.Serialize(redacted); - } + /// Header names, safe values, all bounded — see . + private static string SerializeRedactedHeaders(IReadOnlyDictionary headers) => RejectedDeliveryHeaders.Serialize(headers); /// /// Best-effort delivery id extraction from common provider headers. Returns null if no diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.cs b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.cs index fe76937c3..03a1bf8af 100644 --- a/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.cs +++ b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookIngestionService.cs @@ -33,15 +33,17 @@ public sealed partial class WebhookIngestionService : IWebhookIngestionService, private readonly IPayloadEncryptor _encryptor; private readonly IMediator _mediator; private readonly IIngestionAuditor _auditor; + private readonly IWebhookClaimStore _claims; private readonly ILogger _logger; - public WebhookIngestionService(CodeSpaceDbContext db, IProviderRegistry registry, IPayloadEncryptor encryptor, IMediator mediator, IIngestionAuditor auditor, ILogger logger) + public WebhookIngestionService(CodeSpaceDbContext db, IProviderRegistry registry, IPayloadEncryptor encryptor, IMediator mediator, IIngestionAuditor auditor, IWebhookClaimStore claims, ILogger logger) { _db = db; _registry = registry; _encryptor = encryptor; _mediator = mediator; _auditor = auditor; + _claims = claims; _logger = logger; } @@ -59,6 +61,7 @@ public async Task IngestAsync(Guid webhookId, string body, IReadOnlyDictionary + /// A body signed by a provider whose signature covers the body only () stays valid + /// for as long as the secret does, so the hook claims it, held by its delivery id, for . The + /// same body under ANOTHER id is a replay of a captured delivery and starts nothing; under the SAME id it is the + /// provider redelivering, which goes on to the per-activation idempotency key exactly as before — so an operator who + /// fixes a trigger and presses Redeliver still gets their run. Claimed only for a body that normalised into an event: + /// nothing else can start a run, and most deliveries are events nothing acts on. + /// + private async Task ClaimBodyOrAuditReplayAsync(IngestionSubject subject, string body, string deliveryId, IReadOnlyDictionary headers, CancellationToken cancellationToken) + { + if (!_registry.TryGet(subject.Provider, out _)) return true; + + var claimKey = BuildBodyClaimKey(subject.WebhookId, body); + + if (await _claims.TryClaimAsync(claimKey, deliveryId, ReplayWindow, cancellationToken).ConfigureAwait(false)) return true; + + await AuditReplayedDeliveryAsync(subject, claimKey, deliveryId, headers, cancellationToken).ConfigureAwait(false); + return false; + } + + /// One claim per (hook, body): the SHA-256 of the exact bytes the signature covered, never the body itself. + internal static string BuildBodyClaimKey(Guid webhookId, string body) => + $"body:{webhookId:N}:{Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(body))).ToLowerInvariant()}"; + /// /// Every way a reader can fail on an untrusted body: JsonException (not JSON), /// KeyNotFoundException (missing GetProperty), InvalidOperationException (GetString/GetBoolean diff --git a/backend/src/CodeSpace.Core/Services/Webhooks/WebhookLookup.cs b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookLookup.cs new file mode 100644 index 000000000..dba8ec32c --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Webhooks/WebhookLookup.cs @@ -0,0 +1,18 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; + +namespace CodeSpace.Core.Services.Webhooks; + +/// A primary-key probe of the table the scope names. Inactive and retired hooks exist: their refusal needs the delivery's headers, so ingestion makes it. +public sealed class WebhookLookup : IWebhookLookup, IScopedDependency +{ + private readonly CodeSpaceDbContext _db; + + public WebhookLookup(CodeSpaceDbContext db) { _db = db; } + + public async Task ExistsAsync(ProviderWebhookScope scope, Guid webhookId, CancellationToken cancellationToken) => scope == ProviderWebhookScope.Connection + ? await _db.ConnectionWebhook.AsNoTracking().AnyAsync(w => w.Id == webhookId, cancellationToken).ConfigureAwait(false) + : await _db.RepositoryWebhook.AsNoTracking().AnyAsync(w => w.Id == webhookId, cancellationToken).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/PrTriggerSchemas.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/PrTriggerSchemas.cs index e6c6dcfeb..84c2b0a3d 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/PrTriggerSchemas.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/PrTriggerSchemas.cs @@ -1,9 +1,9 @@ namespace CodeSpace.Core.Services.Workflows.Nodes.Builtin; /// -/// Shared schema constants for PR-trigger nodes. Both and -/// configure the same repositories filter array — -/// keeping it here ensures they never drift and makes drift detection trivially testable. +/// Shared schema constants for PR-trigger nodes. Every PR trigger configures the same repositories filter array; +/// the two an outsider can cause — and — add the +/// authors filter on top. Kept here so they never drift, which makes drift detection trivially testable. /// internal static class PrTriggerSchemas { @@ -43,4 +43,55 @@ internal static class PrTriggerSchemas "additionalProperties": false } """; + + /// + /// plus authors: who may start a run with a pull request. No default + /// on purpose — an activation naming none takes its repository's default (members only when the provider says it is + /// public or internal), and a schema default would be written into every config the editor saves, pinning today's + /// guess forever. + /// Read by PullRequestTriggerAuthors. + /// + internal const string OutsiderReachableConfigSchemaJson = """ + { + "type": "object", + "properties": { + "repositories": { + "type": "array", + "x-selector": "trigger.repositories", + "default": [], + "description": "Each row = one repo + its required labels (AND match).", + "items": { + "type": "object", + "properties": { + "repositoryId": { + "type": "string", + "format": "uuid", + "description": "Repository to match." + }, + "labels": { + "type": "array", + "items": { "type": "string" }, + "description": "PR must carry every label listed (case-sensitive)." + } + }, + "required": ["repositoryId"], + "additionalProperties": false + } + }, + "authors": { + "type": "string", + "enum": ["any", "members"], + "title": "Pull requests from", + "description": "Left unset: members only on a public or internal repository, anyone on a private one.", + "x-control": "radioCards", + "x-enumLabels": { "any": "Anyone", "members": "Members only" }, + "x-optionConsequence": { + "any": "Any author starts a run, including a fork PR from someone outside the repository.", + "members": "Only an author with a role on the repository (GitHub owner, member or collaborator; GitLab Developer or above) starts a run, and new commits only when the author or someone with a role pushed them." + } + } + }, + "additionalProperties": false + } + """; } diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrOpenedNode.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrOpenedNode.cs index 03a2914ab..e37327202 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrOpenedNode.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrOpenedNode.cs @@ -25,7 +25,7 @@ public sealed class TriggerPrOpenedNode : INodeRuntime Kind = NodeKind.Trigger, IconKey = "git-pull-request", Description = "Starts the workflow when a pull/merge request is opened.", - ConfigSchema = SchemaBuilder.Parse(PrTriggerSchemas.RepositoriesConfigSchemaJson), + ConfigSchema = SchemaBuilder.Parse(PrTriggerSchemas.OutsiderReachableConfigSchemaJson), InputSchema = SchemaBuilder.EmptyObject(), OutputSchema = SchemaBuilder.Parse(""" { @@ -40,7 +40,10 @@ public sealed class TriggerPrOpenedNode : INodeRuntime "authorName": { "type": "string" }, "webUrl": { "type": "string" }, "labels": { "type": "array", "items": { "type": "string" } }, - "isDraft": { "type": "boolean" } + "isDraft": { "type": "boolean" }, + "authorAssociation": { "type": "string", "enum": ["member", "contributor", "none", "unknown"] }, + "isFork": { "type": "boolean" }, + "headRepositoryFullName": { "type": ["string","null"] } } } """) diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrUpdatedNode.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrUpdatedNode.cs index ccae9e8df..fea661c0a 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrUpdatedNode.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/TriggerPrUpdatedNode.cs @@ -14,7 +14,7 @@ public sealed class TriggerPrUpdatedNode : INodeRuntime Kind = NodeKind.Trigger, IconKey = "git-commit-horizontal", Description = "Starts the workflow when new commits are pushed to a pull/merge request.", - ConfigSchema = SchemaBuilder.Parse(PrTriggerSchemas.RepositoriesConfigSchemaJson), + ConfigSchema = SchemaBuilder.Parse(PrTriggerSchemas.OutsiderReachableConfigSchemaJson), InputSchema = SchemaBuilder.EmptyObject(), OutputSchema = SchemaBuilder.Parse(""" { @@ -25,7 +25,10 @@ public sealed class TriggerPrUpdatedNode : INodeRuntime "previousHeadSha": { "type": "string" }, "newHeadSha": { "type": "string" }, "labels": { "type": "array", "items": { "type": "string" } }, - "isDraft": { "type": "boolean" } + "isDraft": { "type": "boolean" }, + "authorAssociation": { "type": "string", "enum": ["member", "contributor", "none", "unknown"] }, + "isFork": { "type": "boolean" }, + "headRepositoryFullName": { "type": ["string","null"] } } } """) diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestAuthorRefusedException.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestAuthorRefusedException.cs new file mode 100644 index 000000000..d93f71cba --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestAuthorRefusedException.cs @@ -0,0 +1,20 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Core.Services.Workflows.RunSources.Admission.Exceptions; + +/// An activation that admits only members saw a pull request whose author — or whoever pushed its new commits — is not one. Costs that activation's run only. +public sealed class PullRequestAuthorRefusedException : Exception, IFailure +{ + public PullRequestAuthorRefusedException(string detail, string auditKey) : base(detail) + { + AuditKey = auditKey; + } + + /// What the refusal row is deduplicated on — one per (activation, PR) per day, however many deliveries the author sends. + public string AuditKey { get; } + + /// The user is known and their standing is not enough: the same user's next pull request or push is refused the same way. + public FailureKind Kind => FailureKind.Forbidden; + + public string Code => FailureCodes.Forbidden; +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestTriggerDebouncedException.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestTriggerDebouncedException.cs new file mode 100644 index 000000000..207cdf272 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/Exceptions/PullRequestTriggerDebouncedException.cs @@ -0,0 +1,21 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Core.Services.Workflows.RunSources.Admission.Exceptions; + +/// The same activation started a run for this pull request at this head commit inside the debounce window. Costs that activation's run only. +public sealed class PullRequestTriggerDebouncedException : Exception, IFailure +{ + public PullRequestTriggerDebouncedException(int number, string? headSha, TimeSpan window, string auditKey) + : base($"pull request #{number} at {headSha ?? "an unreported head"} already started a run from this trigger in the last {window.TotalSeconds:0} seconds") + { + AuditKey = auditKey; + } + + /// What the refusal row is deduplicated on — one per (activation, PR, head) per day, however many deliveries arrive. + public string AuditKey { get; } + + /// A window that is spent, not a rule that refuses: the same head starts a run again once it passes, and a new head at once. + public FailureKind Kind => FailureKind.Exhausted; + + public string Code => FailureCodes.RateLimited; +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/IPullRequestTriggerAdmission.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/IPullRequestTriggerAdmission.cs new file mode 100644 index 000000000..b5a337e30 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/IPullRequestTriggerAdmission.cs @@ -0,0 +1,21 @@ +using System.Text.Json; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Events; + +namespace CodeSpace.Core.Services.Workflows.RunSources.Admission; + +/// +/// The two questions a pull-request trigger an outsider can cause has to answer after its matcher said yes and before +/// a run starts: may THIS author — and whoever pushed its new commits — start a run here, and has this activation not +/// just started one for this PR at this head commit. +/// +public interface IPullRequestTriggerAdmission +{ + /// + /// Returns when the run may start — and always for an event no outsider can cause. Throws + /// or , + /// each costing only this activation's run. Completes the event's author association on the way, so the run's + /// payload says who wrote the PR whichever filter the activation chose. + /// + Task EnsureAdmittedAsync(WorkflowActivation activation, NormalizedEvent normalizedEvent, JsonElement activationConfig, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAdmission.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAdmission.cs new file mode 100644 index 000000000..8dc4c2609 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAdmission.cs @@ -0,0 +1,180 @@ +using System.Text.Json; +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Providers; +using CodeSpace.Core.Services.Providers.Capabilities; +using CodeSpace.Core.Services.Webhooks; +using CodeSpace.Core.Services.Workflows.RunSources.Admission.Exceptions; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Events; +using CodeSpace.Messages.Events.PullRequest; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Workflows.RunSources.Admission; + +/// +/// Admits or refuses one activation's run for a pull request an outsider can cause — opened, reopened, new commits. +/// A fork PR from a stranger on a public repository used to start a run like any member's, and every close/reopen +/// started another. Now the activation's authors filter decides the first — on the author, and on whoever pushed new +/// commits — and a short per-(activation, PR, head) debounce the second. Every refusal costs only this activation's run. +/// +public sealed class PullRequestTriggerAdmission : IPullRequestTriggerAdmission, IScopedDependency +{ + /// + /// How long one run from an activation holds off the next for the same PR at the same head commit. A close/reopen loop + /// re-sends a head that already ran, and that is what this holds off; a push moves the head, so new commits always + /// start their own run, however soon after the last. Leading-edge: the first event for a head is the one that runs. + /// + public static readonly TimeSpan DebounceWindow = TimeSpan.FromSeconds(60); + + /// How long dispatch waits on the provider for a user's standing. Webhook senders time out at about ten seconds, and an unanswered lookup reads as not a member. + internal static readonly TimeSpan AuthorLookupTimeout = TimeSpan.FromSeconds(5); + + private readonly CodeSpaceDbContext _db; + private readonly IProviderRegistry _providers; + private readonly IWebhookClaimStore _claims; + private readonly ILogger _logger; + + /// The event whose users this scope has asked the provider about, and their answers — once per user per delivery, however many activations match it. + private IPullRequestOriginEvent? _standingsFor; + private readonly Dictionary _standings = new(); + + public PullRequestTriggerAdmission(CodeSpaceDbContext db, IProviderRegistry providers, IWebhookClaimStore claims, ILogger logger) + { + _db = db; + _providers = providers; + _claims = claims; + _logger = logger; + } + + public async Task EnsureAdmittedAsync(WorkflowActivation activation, NormalizedEvent normalizedEvent, JsonElement activationConfig, CancellationToken cancellationToken) + { + if (normalizedEvent is not IPullRequestOriginEvent pullRequest) return; + + await CompleteAuthorAssociationAsync(pullRequest, cancellationToken).ConfigureAwait(false); + + var required = PullRequestTriggerAuthors.Required(activationConfig, pullRequest.Origin); + + EnsureAuthorAdmitted(activation, pullRequest, required); + await EnsurePusherAdmittedAsync(activation, pullRequest, required, cancellationToken).ConfigureAwait(false); + await EnsureNotDebouncedAsync(activation, pullRequest, normalizedEvent.ProviderEventId, cancellationToken).ConfigureAwait(false); + } + + /// One key per (activation, pull request, head): the activation's run for this head holds it for . + internal static string BuildDebounceKey(Guid activationId, Guid repositoryId, int number, string? headSha) => $"pr-debounce:{activationId:N}:{repositoryId:N}:{number}:{headSha}"; + + /// + /// One refusal row per (activation, PR, kind) per day. An outsider's reopen loop is a stream of genuinely signed + /// deliveries, so a row per delivery would bury every other refusal in the operator's list just as forged ones did. + /// + internal static string BuildRefusalAuditKey(string kind, Guid activationId, IPullRequestOriginEvent pullRequest, DateTimeOffset now) => + $"rejected:{kind}:{activationId:N}:{pullRequest.RepositoryId:N}:{pullRequest.Number}:{now.UtcTicks / WebhookIngestionService.RefusalAuditWindow.Ticks}"; + + private void EnsureAuthorAdmitted(WorkflowActivation activation, IPullRequestOriginEvent pullRequest, string required) + { + if (PullRequestTriggerAuthors.Admits(required, pullRequest.Origin.AuthorAssociation)) return; + + throw new PullRequestAuthorRefusedException(AuthorRefusal(pullRequest), BuildRefusalAuditKey("author", activation.Id, pullRequest, DateTimeOffset.UtcNow)); + } + + /// The pusher's standing is asked only when the rule needs it: someone other than the author pushed to a fork head. + private async Task EnsurePusherAdmittedAsync(WorkflowActivation activation, IPullRequestOriginEvent pullRequest, string required, CancellationToken cancellationToken) + { + if (PullRequestTriggerAuthors.AdmitsPusher(required, pullRequest.Origin)) return; + + await CompletePusherAssociationAsync(pullRequest, cancellationToken).ConfigureAwait(false); + + if (PullRequestTriggerAuthors.AdmitsPusher(required, pullRequest.Origin)) return; + + throw new PullRequestAuthorRefusedException(PusherRefusal(pullRequest), BuildRefusalAuditKey("pusher", activation.Id, pullRequest, DateTimeOffset.UtcNow)); + } + + /// + /// The delivery id holds the claim, so two activations' claims for one delivery never collide and a concurrent copy of + /// the same delivery passes here to be deduplicated by the run's idempotency key. A delivery posted again after its run + /// started never gets here: dispatch skips it as already started, so it cannot restart the window. + /// + private async Task EnsureNotDebouncedAsync(WorkflowActivation activation, IPullRequestOriginEvent pullRequest, string deliveryId, CancellationToken cancellationToken) + { + var claimed = await _claims.TryClaimAsync(BuildDebounceKey(activation.Id, pullRequest.RepositoryId, pullRequest.Number, pullRequest.HeadSha), deliveryId, DebounceWindow, cancellationToken).ConfigureAwait(false); + + if (claimed) return; + + throw new PullRequestTriggerDebouncedException(pullRequest.Number, pullRequest.HeadSha, DebounceWindow, $"{BuildRefusalAuditKey("debounce", activation.Id, pullRequest, DateTimeOffset.UtcNow)}:{pullRequest.HeadSha}"); + } + + private static string AuthorRefusal(IPullRequestOriginEvent pullRequest) => + $"pull request #{pullRequest.Number} was written by an author whose standing is '{Describe(pullRequest.Origin.AuthorAssociation)}', and this trigger admits only members — set 'Pull requests from' to Anyone to admit every author"; + + private static string PusherRefusal(IPullRequestOriginEvent pullRequest) => + $"new commits on pull request #{pullRequest.Number} were pushed by user {pullRequest.Origin.PusherExternalId}, not its author, to a head in another repository, and their standing is '{Describe(pullRequest.Origin.PusherAssociation)}' — this trigger admits only members' code; set 'Pull requests from' to Anyone to admit it"; + + private static string Describe(PullRequestAuthorAssociation association) => association.ToString().ToLowerInvariant(); + + /// + /// GitLab's payload names the author but not their standing; ask the provider, once per delivery, when it can be + /// asked. A provider that cannot answer leaves the standing Unknown, which no members-only trigger admits. + /// + private async Task CompleteAuthorAssociationAsync(IPullRequestOriginEvent pullRequest, CancellationToken cancellationToken) + { + if (pullRequest.Origin.AuthorAssociation != PullRequestAuthorAssociation.Unknown || pullRequest.Origin.AuthorExternalId == null) return; + + var standing = await StandingOfAsync(pullRequest, pullRequest.Origin.AuthorExternalId, cancellationToken).ConfigureAwait(false); + + pullRequest.Origin = pullRequest.Origin with { AuthorAssociation = standing }; + } + + /// Neither provider puts the pusher's standing on the payload. GitLab can be asked; GitHub cannot, so its pusher stays Unknown. + private async Task CompletePusherAssociationAsync(IPullRequestOriginEvent pullRequest, CancellationToken cancellationToken) + { + if (pullRequest.Origin.PusherAssociation != PullRequestAuthorAssociation.Unknown || pullRequest.Origin.PusherExternalId == null) return; + + var standing = await StandingOfAsync(pullRequest, pullRequest.Origin.PusherExternalId, cancellationToken).ConfigureAwait(false); + + pullRequest.Origin = pullRequest.Origin with { PusherAssociation = standing }; + } + + /// A user's standing on this event's repository, asked at most once per delivery — an unanswered lookup costs up to its timeout, and every matching activation repeating it would outlast the provider's own. + private async Task StandingOfAsync(IPullRequestOriginEvent pullRequest, string userExternalId, CancellationToken cancellationToken) + { + if (!ReferenceEquals(_standingsFor, pullRequest)) _standings.Clear(); + + _standingsFor = pullRequest; + + if (_standings.TryGetValue(userExternalId, out var known)) return known; + + var standing = await LookUpStandingAsync(pullRequest.RepositoryId, userExternalId, cancellationToken).ConfigureAwait(false); + + _standings[userExternalId] = standing; + return standing; + } + + private async Task LookUpStandingAsync(Guid repositoryId, string userExternalId, CancellationToken cancellationToken) + { + var repository = await _db.Repository.AsNoTracking().Include(r => r.ProviderInstance).Include(r => r.Credential) + .SingleOrDefaultAsync(r => r.Id == repositoryId, cancellationToken).ConfigureAwait(false); + + if (repository?.Credential == null) return PullRequestAuthorAssociation.Unknown; + if (!_providers.TryGet(repository.ProviderInstance.Provider, out var standing) || standing == null) return PullRequestAuthorAssociation.Unknown; + + return await AskProviderAsync(standing, repository, userExternalId, cancellationToken).ConfigureAwait(false); + } + + private async Task AskProviderAsync(IRepositoryMemberStandingCapability standing, Repository repository, string userExternalId, CancellationToken cancellationToken) + { + using var deadline = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + deadline.CancelAfter(AuthorLookupTimeout); + + try + { + return await standing.GetMemberStandingAsync(new ProviderContext(repository.ProviderInstance, repository.Credential!), repository.ToRemoteRepository(), userExternalId, deadline.Token).ConfigureAwait(false); + } + catch (Exception ex) when (!cancellationToken.IsCancellationRequested) + { + _logger.LogWarning(ex, "Could not read the standing of user {UserExternalId} on repository {RepositoryId}; a members-only trigger will not admit them", userExternalId, repository.Id); + return PullRequestAuthorAssociation.Unknown; + } + } +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAuthors.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAuthors.cs new file mode 100644 index 000000000..01fa35042 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Admission/PullRequestTriggerAuthors.cs @@ -0,0 +1,59 @@ +using System.Text.Json; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Events.PullRequest; + +namespace CodeSpace.Core.Services.Workflows.RunSources.Admission; + +/// +/// Who may start a run with a pull request: the activation's authors value, or +/// , read from the same config the matchers read (schema: PrTriggerSchemas.OutsiderReachableConfigSchemaJson). +/// +/// An activation that names neither — every one saved before this filter existed — takes its repository's default, +/// decided by what the provider said on THIS delivery: members only on a public or internal repository, anyone on a +/// private one. When the delivery does not say, members only for a PR from a fork. Internal counts with public because +/// anyone who can read the repository can fork it and open a PR: every signed-in user of a GitLab instance, every member +/// of a GitHub enterprise. So an existing activation keeps admitting everyone only on a private repository. +/// +/// Members means the people who made the PR's code what it is: its author, and for new commits whoever pushed them. +/// A member can open a PR from an outsider's fork branch, and from then on the outsider pushes the code it runs. +/// +internal static class PullRequestTriggerAuthors +{ + public const string ConfigKey = "authors"; + + public const string Any = "any"; + + public const string Members = "members"; + + /// What this activation requires of this PR's author. A value the filter cannot mean admits only members: a typo must never widen it. + public static string Required(JsonElement activationConfig, PullRequestOrigin origin) + { + if (activationConfig.ValueKind != JsonValueKind.Object) return Default(origin); + if (!activationConfig.TryGetProperty(ConfigKey, out var value) || value.ValueKind == JsonValueKind.Null) return Default(origin); + + return value.ValueKind == JsonValueKind.String && value.GetString() == Any ? Any : Members; + } + + /// Members admits a and nothing else — an author whose standing is unknown is not one. + public static bool Admits(string required, PullRequestAuthorAssociation association) => + required == Any || association == PullRequestAuthorAssociation.Member; + + /// + /// Members admits new commits when their pusher is the author (already admitted), or pushed to a head in the target + /// repository (which takes a role there), or is a member. Anyone else pushing to a fork head is not vouched for — + /// including a pusher whose standing the provider cannot say. + /// + public static bool AdmitsPusher(string required, PullRequestOrigin origin) => + required == Any || PushNeedsNoStanding(origin) || origin.PusherAssociation == PullRequestAuthorAssociation.Member; + + /// No push in this event (an open or reopen), the author's own push, or a push to a head that lives in the target repository. + private static bool PushNeedsNoStanding(PullRequestOrigin origin) => + origin.PusherExternalId == null || origin.PusherExternalId == origin.AuthorExternalId || !origin.IsFork; + + private static string Default(PullRequestOrigin origin) => origin.RepositoryVisibility switch + { + RepositoryVisibility.Public or RepositoryVisibility.Internal => Members, + RepositoryVisibility.Private => Any, + _ => origin.IsFork ? Members : Any + }; +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrOpenedMatcher.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrOpenedMatcher.cs index 94528bcf3..b1b616fe2 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrOpenedMatcher.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrOpenedMatcher.cs @@ -42,7 +42,10 @@ public JsonElement BuildPayload(NormalizedEvent normalizedEvent) authorName = opened.AuthorName, webUrl = opened.WebUrl, labels = opened.Labels, - isDraft = opened.IsDraft + isDraft = opened.IsDraft, + authorAssociation = PullRequestOriginPayload.Association(opened.Origin), + isFork = opened.Origin.IsFork, + headRepositoryFullName = opened.Origin.HeadRepositoryFullName }; return JsonSerializer.SerializeToElement(payload); diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrUpdatedMatcher.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrUpdatedMatcher.cs index 1eb34afa8..c449c65f2 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrUpdatedMatcher.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PrUpdatedMatcher.cs @@ -32,7 +32,10 @@ public JsonElement BuildPayload(NormalizedEvent normalizedEvent) previousHeadSha = synced.PreviousHeadSha, newHeadSha = synced.NewHeadSha, labels = synced.Labels, - isDraft = synced.IsDraft + isDraft = synced.IsDraft, + authorAssociation = PullRequestOriginPayload.Association(synced.Origin), + isFork = synced.Origin.IsFork, + headRepositoryFullName = synced.Origin.HeadRepositoryFullName }; return JsonSerializer.SerializeToElement(payload); diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PullRequestOriginPayload.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PullRequestOriginPayload.cs new file mode 100644 index 000000000..1b0e9a658 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/Matchers/PullRequestOriginPayload.cs @@ -0,0 +1,13 @@ +using CodeSpace.Messages.Events.PullRequest; + +namespace CodeSpace.Core.Services.Workflows.RunSources.Matchers; + +/// +/// How a PR's origin reads in a trigger payload, shared by the two triggers an outsider can cause so +/// {{trigger.authorAssociation}} means the same thing on both: member, contributor, none or +/// unknown. +/// +internal static class PullRequestOriginPayload +{ + public static string Association(PullRequestOrigin origin) => origin.AuthorAssociation.ToString().ToLowerInvariant(); +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/RunSourceDispatcher.cs b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/RunSourceDispatcher.cs index 8818c7d74..17e0a47ba 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/RunSources/RunSourceDispatcher.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/RunSources/RunSourceDispatcher.cs @@ -2,6 +2,8 @@ using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Services.Agents.Authority.Exceptions; using CodeSpace.Core.Services.Completion.Exceptions; +using CodeSpace.Core.Services.Workflows.RunSources.Admission; +using CodeSpace.Core.Services.Workflows.RunSources.Admission.Exceptions; using CodeSpace.Core.Middlewares.Transactional; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; @@ -49,9 +51,10 @@ public sealed class RunSourceDispatcher : private readonly Dispatch.IWorkflowRunDispatcher _runDispatcher; private readonly IIngestionAuditor _auditor; private readonly IPostCommitActions _postCommit; + private readonly IPullRequestTriggerAdmission _admission; private readonly ILogger _logger; - public RunSourceDispatcher(CodeSpaceDbContext db, IRunSourceMatcherRegistry matcherRegistry, IRunStarter runStarter, Dispatch.IWorkflowRunDispatcher runDispatcher, IIngestionAuditor auditor, IPostCommitActions postCommit, ILogger logger) + public RunSourceDispatcher(CodeSpaceDbContext db, IRunSourceMatcherRegistry matcherRegistry, IRunStarter runStarter, Dispatch.IWorkflowRunDispatcher runDispatcher, IIngestionAuditor auditor, IPostCommitActions postCommit, IPullRequestTriggerAdmission admission, ILogger logger) { _db = db; _matcherRegistry = matcherRegistry; @@ -59,6 +62,7 @@ public RunSourceDispatcher(CodeSpaceDbContext db, IRunSourceMatcherRegistry matc _runDispatcher = runDispatcher; _auditor = auditor; _postCommit = postCommit; + _admission = admission; _logger = logger; } @@ -159,29 +163,45 @@ await _db.Repository.AsNoTracking() { anyRefused = true; _logger.LogWarning("Webhook activation authority refused. ActivationId={ActivationId} WorkflowId={WorkflowId} TeamId={TeamId} Code={Code} Reason={Reason}", activation.Id, activation.WorkflowId, activation.Workflow.TeamId, ex.Code, ex.Reason); - await AuditRefusalAsync(activation, normalizedEvent, ex.Code, ex.Reason, "authority", cancellationToken).ConfigureAwait(false); + await AuditRefusalAsync(activation, normalizedEvent, ex.Code, ex.Reason, PerDeliveryRefusalKey("authority", activation, normalizedEvent), cancellationToken).ConfigureAwait(false); } catch (CompletionAdmissionRefusedException ex) { anyRefused = true; _logger.LogWarning("Webhook activation completion admission refused. ActivationId={ActivationId} WorkflowId={WorkflowId} TeamId={TeamId} Reason={Reason}", activation.Id, activation.WorkflowId, activation.Workflow.TeamId, ex.Message); - await AuditRefusalAsync(activation, normalizedEvent, WorkflowRunRequestRejectionReasons.CompletionAdmissionRefused, ex.Message, "admission", cancellationToken).ConfigureAwait(false); + await AuditRefusalAsync(activation, normalizedEvent, WorkflowRunRequestRejectionReasons.CompletionAdmissionRefused, ex.Message, PerDeliveryRefusalKey("admission", activation, normalizedEvent), cancellationToken).ConfigureAwait(false); + } + catch (PullRequestAuthorRefusedException ex) + { + anyRefused = true; + _logger.LogInformation("Webhook activation refused a pull request author. ActivationId={ActivationId} WorkflowId={WorkflowId} TeamId={TeamId} Reason={Reason}", activation.Id, activation.WorkflowId, activation.Workflow.TeamId, ex.Message); + await AuditRefusalAsync(activation, normalizedEvent, WorkflowRunRequestRejectionReasons.AuthorNotMember, ex.Message, ex.AuditKey, cancellationToken).ConfigureAwait(false); + } + catch (PullRequestTriggerDebouncedException ex) + { + anyRefused = true; + _logger.LogInformation("Webhook activation debounced a pull request. ActivationId={ActivationId} WorkflowId={WorkflowId} TeamId={TeamId} Reason={Reason}", activation.Id, activation.WorkflowId, activation.Workflow.TeamId, ex.Message); + await AuditRefusalAsync(activation, normalizedEvent, WorkflowRunRequestRejectionReasons.PullRequestDebounced, ex.Message, ex.AuditKey, cancellationToken).ConfigureAwait(false); } } return (firedRunIds, anyRefused); } - /// One Rejected row per (activation, delivery, refusal kind), so a provider's redelivery of the same refused event collapses onto it. - private async Task AuditRefusalAsync(WorkflowActivation activation, NormalizedEvent normalizedEvent, string reason, string detail, string refusalKind, CancellationToken cancellationToken) => + /// One Rejected row per , so a provider's redelivery of the same refused event collapses onto it. + private async Task AuditRefusalAsync(WorkflowActivation activation, NormalizedEvent normalizedEvent, string reason, string detail, string dedupKey, CancellationToken cancellationToken) => await _auditor.WriteWebhookRejectedAsync(new WebhookRejectionContext { TeamId = activation.Workflow.TeamId, RepositoryId = normalizedEvent.RepositoryId, SourceType = activation.TypeKey, ExternalEventId = normalizedEvent.ProviderEventId, Reason = reason, Detail = $"Activation {activation.Id} workflow {activation.WorkflowId}: {detail}", - DedupKey = $"rejected:{refusalKind}:{activation.Id:N}:{normalizedEvent.ProviderEventId}", + DedupKey = dedupKey, }, cancellationToken).ConfigureAwait(false); + /// One row per (activation, delivery, refusal kind) — a refusal that is the activation's own property and rare. + private static string PerDeliveryRefusalKey(string refusalKind, WorkflowActivation activation, NormalizedEvent normalizedEvent) => + $"rejected:{refusalKind}:{activation.Id:N}:{normalizedEvent.ProviderEventId}"; + /// /// Dispatch each matched run AFTER commit. RunAfterCommitAsync defers into the post-commit drain /// while a transaction is open (the ReceiveWebhookCommand path), so a worker can't pick up a @@ -220,6 +240,11 @@ private static bool CanHandle(IRunSourceMatcher matcher, NormalizedEvent normali } } + /// + /// Ordered by id: firing an activation can take a row lock that is held until the delivery commits (the pull-request + /// debounce claim), so two concurrent deliveries must visit activations in the same order or each could hold what the + /// other waits for. + /// private async Task> LoadActiveActivationsAsync(Guid teamId, IReadOnlyList typeKeys, CancellationToken cancellationToken) { return await _db.WorkflowActivation @@ -230,6 +255,7 @@ private async Task> LoadActiveActivationsAsync && a.DeletedDate == null && a.Workflow.Enabled && a.Workflow.DeletedDate == null) + .OrderBy(a => a.Id) .ToListAsync(cancellationToken).ConfigureAwait(false); } @@ -239,6 +265,12 @@ private async Task> LoadActiveActivationsAsync if (!matcher.Match(normalizedEvent, config)) return null; + var idempotencyKey = SynthesiseProviderIdempotencyKey(matcher.TypeKey, normalizedEvent.ProviderEventId, activation.Id); + + if (await IsAlreadyStartedAsync(idempotencyKey, cancellationToken).ConfigureAwait(false)) return SkipDuplicate(activation, normalizedEvent); + + await _admission.EnsureAdmittedAsync(activation, normalizedEvent, config, cancellationToken).ConfigureAwait(false); + var payload = matcher.BuildPayload(normalizedEvent); // Hand the envelope to the unified starter. The starter stages workflow_run_request + @@ -263,16 +295,10 @@ private async Task> LoadActiveActivationsAsync ActivationId = activation.Id, ActivationSnapshotJson = ActivationAuthoritySnapshot.Serialize(activation), ExternalEventId = normalizedEvent.ProviderEventId, - IdempotencyKey = SynthesiseProviderIdempotencyKey(matcher.TypeKey, normalizedEvent.ProviderEventId, activation.Id), + IdempotencyKey = idempotencyKey, }, cancellationToken).ConfigureAwait(false); - if (runId == Guid.Empty) - { - _logger.LogInformation( - "Skipped duplicate provider delivery: workflow {WorkflowId} activation {ActivationId} delivery {DeliveryId}", - activation.WorkflowId, activation.Id, normalizedEvent.ProviderEventId); - return null; - } + if (runId == Guid.Empty) return SkipDuplicate(activation, normalizedEvent); _logger.LogInformation( "Fired workflow {WorkflowId} via activation {TypeKey} → run {RunId}", @@ -280,6 +306,22 @@ private async Task> LoadActiveActivationsAsync return runId; } + /// + /// A delivery this activation already started a run for — the provider redelivering it, an operator pressing + /// Redeliver, a captured body posted again under its own id — is a duplicate before it is anything else. Answered + /// here, ahead of admission, it cannot take the pull-request debounce claim a second time (which would restart the + /// window and swallow the next real event) or send a provider another standing lookup. Two copies racing each other + /// both pass this read; RunStarter's unique index still keeps one. + /// + private async Task IsAlreadyStartedAsync(string idempotencyKey, CancellationToken cancellationToken) => + await _db.WorkflowRunRequest.AsNoTracking().AnyAsync(r => r.IdempotencyKey == idempotencyKey, cancellationToken).ConfigureAwait(false); + + private Guid? SkipDuplicate(WorkflowActivation activation, NormalizedEvent normalizedEvent) + { + _logger.LogInformation("Skipped duplicate provider delivery: workflow {WorkflowId} activation {ActivationId} delivery {DeliveryId}", activation.WorkflowId, activation.Id, normalizedEvent.ProviderEventId); + return null; + } + /// /// Composite idempotency key for a provider event: {sourceType}:{deliveryId}:{activationId}. /// Lets the SAME delivery fan out to N activations (each gets a unique key) while a diff --git a/backend/src/CodeSpace.Messages/Commands/Webhooks/PurgeExpiredWebhookClaimsCommand.cs b/backend/src/CodeSpace.Messages/Commands/Webhooks/PurgeExpiredWebhookClaimsCommand.cs new file mode 100644 index 000000000..52f566d0c --- /dev/null +++ b/backend/src/CodeSpace.Messages/Commands/Webhooks/PurgeExpiredWebhookClaimsCommand.cs @@ -0,0 +1,20 @@ +using CodeSpace.Messages.Mediation; + +namespace CodeSpace.Messages.Commands.Webhooks; + +/// +/// Delete every webhook_claim row whose window has ended — replayed-body claims after their retention, pull-request +/// debounce claims after their minute. A claim on the same key takes an expired row over in place, so nothing depends on +/// this for correctness; it only keeps the table near one window of deliveries. Dispatched by the recurring sweep. +/// +/// NOT transactional (): each batch commits on its own, so a backlog never +/// holds a long lock that a delivery's claim could queue behind, and one batch failing keeps what the others cleared. +/// NOT tenant-scoped — a system-wide sweep with no actor context. +/// +public sealed record PurgeExpiredWebhookClaimsCommand : ICommand, INonTransactionalCommand; + +/// How many expired claims this sweep deleted — surfaced in the job log so an operator can see it working. +public sealed record PurgeExpiredWebhookClaimsResponse +{ + public required int Deleted { get; init; } +} diff --git a/backend/src/CodeSpace.Messages/Constants/WorkflowRunRequestRejectionReasons.cs b/backend/src/CodeSpace.Messages/Constants/WorkflowRunRequestRejectionReasons.cs index 241fb956e..91a2d8c58 100644 --- a/backend/src/CodeSpace.Messages/Constants/WorkflowRunRequestRejectionReasons.cs +++ b/backend/src/CodeSpace.Messages/Constants/WorkflowRunRequestRejectionReasons.cs @@ -58,4 +58,32 @@ public static class WorkflowRunRequestRejectionReasons /// against the repository so its own Webhook tab says why its deliveries are refused. /// public const string RepositoryOutsideHookOwner = "repository_outside_hook_owner"; + + /// + /// A correctly signed delivery from a provider that stamps every delivery with an id (GitHub's X-GitHub-Delivery) + /// arrived without one. A real provider never sends that; a tool replaying a captured body does, because without an + /// id every post of the same body would read as a new delivery. + /// + public const string DeliveryIdMissing = "delivery_id_missing"; + + /// + /// This hook already accepted the identical signed body under a different delivery id. The signature covers the body + /// only, so a captured body stays valid for as long as the secret does; a fresh id on an old body is a replay, not a + /// new event. A provider redelivery keeps its id and is not refused. + /// + public const string DeliveryReplayed = "delivery_replayed"; + + /// + /// A pull-request trigger that admits only members — explicitly, or by default on a public or internal repository — + /// saw a PR whose author holds no role there, or new commits pushed to a fork head by someone other than the author + /// who holds none. Started nothing for that activation; every other matching activation still ran. + /// + public const string AuthorNotMember = "author_not_member"; + + /// + /// The same activation already started a run for this pull request at the same head commit moments ago. Events for + /// one head within the debounce window start one run, not one per event, so closing and reopening a PR cannot be used + /// to start runs on demand; a push moves the head, so new commits are never held back. + /// + public const string PullRequestDebounced = "pull_request_debounced"; } diff --git a/backend/src/CodeSpace.Messages/Enums/PullRequestAuthorAssociation.cs b/backend/src/CodeSpace.Messages/Enums/PullRequestAuthorAssociation.cs new file mode 100644 index 000000000..abc27a345 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Enums/PullRequestAuthorAssociation.cs @@ -0,0 +1,21 @@ +namespace CodeSpace.Messages.Enums; + +/// +/// How a pull request's AUTHOR stands with the repository it targets — the one fact that tells a member's change +/// from an outsider's. Normalised across providers: GitHub reports it on the payload (author_association); +/// GitLab does not, so it is the author's project access level, looked up when a trigger needs it. +/// +public enum PullRequestAuthorAssociation +{ + /// Not reported, or the lookup failed. Never trusted as a member. + Unknown, + + /// No standing: GitHub NONE / MANNEQUIN, or a GitLab user with no role or a role below Developer. + None, + + /// Has had changes merged before but holds no role: GitHub CONTRIBUTOR / FIRST_TIME_CONTRIBUTOR / FIRST_TIMER. + Contributor, + + /// Holds a role on the repository or its owner: GitHub OWNER / MEMBER / COLLABORATOR; GitLab Developer or above. + Member +} diff --git a/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOpenedEvent.cs b/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOpenedEvent.cs index adf66ebb8..5e7470698 100644 --- a/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOpenedEvent.cs +++ b/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOpenedEvent.cs @@ -1,6 +1,6 @@ namespace CodeSpace.Messages.Events.PullRequest; -public sealed class PullRequestOpenedEvent : NormalizedEvent +public sealed class PullRequestOpenedEvent : NormalizedEvent, IPullRequestOriginEvent { public required string ExternalPullRequestId { get; init; } public required int Number { get; init; } @@ -12,6 +12,9 @@ public sealed class PullRequestOpenedEvent : NormalizedEvent public required string AuthorName { get; init; } public required string WebUrl { get; init; } + /// The head commit when the PR was opened or reopened. Provider source: GitHub pull_request.head.sha; GitLab object_attributes.last_commit.id. Null when the payload omits it. + public string? HeadSha { get; init; } + /// /// Label names attached to the PR at the moment the webhook fired. Provider source: /// GitHub pull_request.labels[].name; GitLab top-level labels[].title. @@ -30,4 +33,7 @@ public sealed class PullRequestOpenedEvent : NormalizedEvent /// level — mirroring how GitHub/GitLab deliver the event and let the consumer filter. /// public bool IsDraft { get; init; } + + /// Who wrote the PR and where its head lives — see . Defaults to nothing known. + public PullRequestOrigin Origin { get; set; } = new(); } diff --git a/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOrigin.cs b/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOrigin.cs new file mode 100644 index 000000000..c2ffe080c --- /dev/null +++ b/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestOrigin.cs @@ -0,0 +1,52 @@ +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Messages.Events.PullRequest; + +/// +/// Who wrote a pull request and where its head lives — what a trigger needs to tell a member's change from an +/// outsider's. A fork's head branch can carry the same name as the base branch, so without these a workflow sees +/// sourceBranch: main and cannot know the code came from someone else's repository. +/// +public sealed record PullRequestOrigin +{ + /// The PR author's provider user id — the user describes. Not the actor of this delivery: a maintainer reopening an outsider's PR is not its author. + public string? AuthorExternalId { get; init; } + + public PullRequestAuthorAssociation AuthorAssociation { get; init; } = PullRequestAuthorAssociation.Unknown; + + /// The head branch lives in another repository than the one the PR targets. False when the payload does not say. + public bool IsFork { get; init; } + + /// The head repository's full path (evil/repo). Null when the provider omits it — a deleted fork, or an older payload. + public string? HeadRepositoryFullName { get; init; } + + /// The target repository's visibility as the provider reported it on this delivery. Null when the payload does not carry it. + public RepositoryVisibility? RepositoryVisibility { get; init; } + + /// + /// Who pushed the commits this event carries, by provider user id — GitHub's sender on a synchronize, GitLab's + /// user on an update that moved the head. Null on an open or reopen, which pushes nothing: whoever reopened a PR + /// did not write its code. + /// + public string? PusherExternalId { get; init; } + + /// How stands with the repository. Neither provider says on the payload, so it is Unknown until looked up — and only when a trigger has to decide on it. + public PullRequestAuthorAssociation PusherAssociation { get; init; } = PullRequestAuthorAssociation.Unknown; +} + +/// +/// A pull-request event an outsider can cause — an open, a reopen, a push of new commits — and so the one a trigger's +/// authors filter and debounce apply to. A merge needs a member's hand, so a merged event is not one. +/// +public interface IPullRequestOriginEvent +{ + Guid RepositoryId { get; } + + int Number { get; } + + /// The commit the PR's head is at in this event — what one run of a trigger is FOR. Null when the payload does not say. + string? HeadSha { get; } + + /// Settable because GitLab names the author but not their standing: dispatch completes the association before deciding on it. + PullRequestOrigin Origin { get; set; } +} diff --git a/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestSynchronizedEvent.cs b/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestSynchronizedEvent.cs index 8688c2ed6..ada0819de 100644 --- a/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestSynchronizedEvent.cs +++ b/backend/src/CodeSpace.Messages/Events/PullRequest/PullRequestSynchronizedEvent.cs @@ -1,6 +1,6 @@ namespace CodeSpace.Messages.Events.PullRequest; -public sealed class PullRequestSynchronizedEvent : NormalizedEvent +public sealed class PullRequestSynchronizedEvent : NormalizedEvent, IPullRequestOriginEvent { public required string ExternalPullRequestId { get; init; } public required int Number { get; init; } @@ -23,4 +23,9 @@ public sealed class PullRequestSynchronizedEvent : NormalizedEvent /// Surfaced as {{trigger.isDraft}} so a workflow can gate on it. /// public bool IsDraft { get; init; } + + /// Who wrote the PR and where its head lives — see . Defaults to nothing known. + public PullRequestOrigin Origin { get; set; } = new(); + + string? IPullRequestOriginEvent.HeadSha => NewHeadSha; } diff --git a/backend/src/CodeSpace.Messages/Queries/Webhooks/WebhookExistsQuery.cs b/backend/src/CodeSpace.Messages/Queries/Webhooks/WebhookExistsQuery.cs new file mode 100644 index 000000000..93cd37709 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Queries/Webhooks/WebhookExistsQuery.cs @@ -0,0 +1,17 @@ +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Mediation; + +namespace CodeSpace.Messages.Queries.Webhooks; + +/// +/// Whether a webhook id names a hook at all. ANONYMOUS, like the delivery it precedes: it answers only what that +/// delivery's own 404 already tells the caller, and it is asked first so that a body posted to an id naming nothing is +/// never read. +/// +public sealed record WebhookExistsQuery : IQuery +{ + public required Guid WebhookId { get; init; } + + /// Which table the id lives in: a per-repository hook, or a group / organization (connection) hook. + public required ProviderWebhookScope Scope { get; init; } +} diff --git a/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookIngressSourceLimitE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookIngressSourceLimitE2ETests.cs new file mode 100644 index 000000000..cff3f3c77 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookIngressSourceLimitE2ETests.cs @@ -0,0 +1,91 @@ +using System.Collections.Concurrent; +using System.Diagnostics; +using System.Net; +using CodeSpace.Api.Extensions; +using CodeSpace.E2ETests.Infrastructure; +using Shouldly; + +namespace CodeSpace.E2ETests.Webhooks; + +/// +/// What one unauthenticated source can make the ingress rate limiter HOLD. The per-target limiter keys on the hook id or +/// callback token in the URL — values the sender chooses — so a sender changing the target on every request used to get +/// a fresh limiter each time: never limited, and each limiter kept alive past its window (about 34 KB apiece for a +/// kilobytes-long token). These are the two bounds on that: a per-address budget that runs before any per-target limiter +/// is created, and routes that refuse a target no hook or minted token can be. +/// +/// Tier: 🟢 High-fidelity — real app host (), real routing, the production +/// RateLimiterOptions from WebhookIngressRateLimitExtension, the real controllers and their lookups against +/// real Postgres. Its own class, and so its own host: spending one address's whole budget here must not throttle the +/// sibling tests that share WebhookOutsiderHardeningE2ETests' host (TestServer sends every request from the same +/// unknown address). +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class WebhookIngressSourceLimitE2ETests : IClassFixture +{ + /// Well inside the one-minute window, so the budget cannot refill mid-test and pass the final request for the wrong reason. + private static readonly TimeSpan BudgetSpendDeadline = TimeSpan.FromSeconds(45); + + private readonly WebhookApiFactory _factory; + + public WebhookIngressSourceLimitE2ETests(WebhookApiFactory factory) { _factory = factory; } + + [Fact] + public async Task One_source_changing_the_target_on_every_request_is_limited_by_its_address() + { + var client = _factory.CreateClient(); + var statuses = new ConcurrentDictionary(); + var clock = Stopwatch.StartNew(); + + await Parallel.ForEachAsync(Enumerable.Range(0, WebhookIngressRateLimitExtension.SourcePermitsPerWindow), new ParallelOptions { MaxDegreeOfParallelism = 16 }, async (i, ct) => + { + var response = await client.PostAsync(RotatingTarget(i), new StringContent("{}"), ct); + statuses.AddOrUpdate(response.StatusCode, 1, (_, n) => n + 1); + }); + + clock.Elapsed.ShouldBeLessThan(BudgetSpendDeadline, "the budget must be spent inside one window or the final request below proves nothing; a slow host here is the thing to look at"); + statuses.Keys.ShouldBe(new[] { HttpStatusCode.NotFound }, ignoreOrder: true, customMessage: $"every request inside the address's budget must reach its endpoint and be told the target does not exist; got {string.Join(", ", statuses.Select(s => $"{(int)s.Key}x{s.Value}"))}"); + + var over = await client.PostAsync(RotatingTarget(WebhookIngressRateLimitExtension.SourcePermitsPerWindow), new StringContent("{}")); + + over.StatusCode.ShouldBe(HttpStatusCode.TooManyRequests, customMessage: "a sender that changes the target every time must still run out — check the GlobalLimiter in WebhookIngressRateLimitExtension; without it each new target is a new limiter and nothing ever answers 429"); + } + + [Fact] + public async Task A_callback_token_no_minted_token_can_be_never_reaches_the_limiter_and_leaves_nothing_behind() + { + // The measured shape: a fresh 8000-character token per request kept ~34 KB alive per request for about 70 s. + const int requests = 1000; + var client = _factory.CreateClient(); + await client.PostAsync($"/api/workflows/callbacks/{new string('w', 8000)}", new StringContent("{}")); // warm the fallback path + + var before = GC.GetTotalMemory(forceFullCollection: true); + + for (var i = 0; i < requests; i++) + { + var response = await client.PostAsync($"/api/workflows/callbacks/{RandomToken(8000)}", new StringContent("{}")); + response.StatusCode.ShouldBe(HttpStatusCode.NotFound, customMessage: "an over-long token matches no callback route, so it is answered by the fallback and never by the limiter or the action"); + } + + var retained = GC.GetTotalMemory(forceFullCollection: true) - before; + + retained.ShouldBeLessThan(10L * 1024 * 1024, $"{requests} requests with distinct 8000-character tokens retained {retained / 1024} KB; before the route bounded the token they retained about 34 MB — check the {{token:length(32)}} constraint on WorkflowCallbacksController.Resume"); + } + + /// A target nobody has registered, spread across all three ingress routes so the budget is shown to be per address and not per route. + private static string RotatingTarget(int i) => (i % 3) switch + { + 0 => $"/api/webhooks/{Guid.NewGuid()}", + 1 => $"/api/webhooks/connection/{Guid.NewGuid()}", + _ => $"/api/workflows/callbacks/{Guid.NewGuid():N}" + }; + + private static string RandomToken(int length) + { + const string alphabet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + var chars = new char[length]; + for (var i = 0; i < length; i++) chars[i] = alphabet[Random.Shared.Next(alphabet.Length)]; + return new string(chars); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs new file mode 100644 index 000000000..5e00f51c0 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs @@ -0,0 +1,509 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Security.Cryptography; +using System.Text; +using System.Text.Json; +using CodeSpace.Api.Controllers; +using CodeSpace.Api.Extensions; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Workflows.Engine; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.E2ETests.Webhooks; + +/// +/// What an outsider can make the webhook endpoints do, on the WIRE. Each case is a request a stranger can send — a fork PR +/// on a public repository, a close/reopen loop, a captured signed body posted again, a forged signature, an oversized or +/// unaddressed body, a flood — and the status, runs and refusal rows it must leave behind. +/// +/// Tier: 🟢 High-fidelity — real app host (), real routing, rate limiter, controller, +/// MediatR transaction, signature verifiers, normalizers, RunSourceDispatcher and RunStarter, against real +/// Postgres. The GitLab case answers the real GitLab provider class over the wire from a loopback +/// . Only the Hangfire client is a no-op, so a started run is a row and never executes. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class WebhookOutsiderHardeningE2ETests : IClassFixture +{ + private readonly WebhookApiFactory _factory; + + public WebhookOutsiderHardeningE2ETests(WebhookApiFactory factory) { _factory = factory; } + + // ─── Who may start a run ─────────────────────────────────────────────────── + + [Fact] + public async Task An_outsiders_fork_pr_on_a_public_repository_starts_nothing_by_default_and_says_why() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Public); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", "{}"); + + var status = await PostGitHubAsync(seed, "pull_request", GitHubPr("reopened", number: 7, association: "NONE", headRepo: Fork, visibility: "public")); + + status.ShouldBe(HttpStatusCode.OK); + (await CountRunsAsync(workflow)).ShouldBe(0, customMessage: "An activation saved without an authors choice must admit only members on a public repository. Check PullRequestTriggerAuthors.Required and the admission call in RunSourceDispatcher.FireIfMatchesAsync."); + (await LoadRefusalsAsync(seed)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.AuthorNotMember), customMessage: "the operator must be told why the PR started nothing, not just see silence"); + } + + [Fact] + public async Task A_members_pr_on_a_public_repository_runs_and_its_payload_says_who_wrote_it() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Public); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", "{}"); + + await PostGitHubAsync(seed, "pull_request", GitHubPr("opened", number: 8, association: "MEMBER", headRepo: SameRepository, visibility: "public")); + + var payload = await LoadOnlyRunPayloadAsync(workflow); + payload.GetProperty("authorAssociation").GetString().ShouldBe("member"); + payload.GetProperty("isFork").GetBoolean().ShouldBeFalse(); + payload.GetProperty("headRepositoryFullName").GetString().ShouldBe("acme/api"); + } + + [Fact] + public async Task An_activation_that_admits_any_author_runs_an_outsiders_fork_pr_and_can_see_it_is_one() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Public); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", """{"authors":"any"}"""); + + await PostGitHubAsync(seed, "pull_request", GitHubPr("opened", number: 9, association: "FIRST_TIME_CONTRIBUTOR", headRepo: Fork, visibility: "public")); + + var payload = await LoadOnlyRunPayloadAsync(workflow); + payload.GetProperty("authorAssociation").GetString().ShouldBe("contributor"); + payload.GetProperty("isFork").GetBoolean().ShouldBeTrue(); + payload.GetProperty("headRepositoryFullName").GetString().ShouldBe("evil/repo"); + payload.GetProperty("sourceBranch").GetString().ShouldBe("main", "the fork's head branch is named like the base branch — which is exactly why isFork has to be on the payload"); + } + + [Fact] + public async Task An_existing_activation_on_a_private_repository_still_admits_any_author() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", "{}"); + + await PostGitHubAsync(seed, "pull_request", GitHubPr("opened", number: 10, association: "NONE", headRepo: Fork, visibility: "private")); + + (await CountRunsAsync(workflow)).ShouldBe(1, customMessage: "the default only narrows on PUBLIC repositories; a private repository's activations keep their behaviour"); + } + + [Fact] + public async Task An_existing_activation_on_an_internal_repository_admits_only_members() + { + // Every member of the enterprise can read an internal repository, fork it and open a PR from the fork. + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Internal); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", "{}"); + + await PostGitHubAsync(seed, "pull_request", GitHubPr("opened", number: 16, association: "NONE", headRepo: Fork, visibility: "internal")); + await PostGitHubAsync(seed, "pull_request", GitHubPr("opened", number: 17, association: "MEMBER", headRepo: SameRepository, visibility: "internal")); + + (await CountRunsAsync(workflow)).ShouldBe(1, customMessage: "on an internal repository an activation saved without an authors choice admits the member's PR and not the outsider's — check PullRequestTriggerAuthors.Default"); + (await LoadRefusalsAsync(seed)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.AuthorNotMember)); + } + + [Fact] + public async Task An_outsiders_push_to_a_members_pr_from_the_outsiders_fork_starts_nothing_on_a_public_repository() + { + // GitHub lets a member open a PR from anyone's fork branch; from then on the fork's owner pushes the code it runs. + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Public); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.updated", "{}"); + + (await PostGitHubAsync(seed, "pull_request", GitHubSync(number: 18, after: "sha-evil", association: "MEMBER", headRepo: Fork, sender: SenderOutsider, visibility: "public"))).ShouldBe(HttpStatusCode.OK); + (await PostGitHubAsync(seed, "pull_request", GitHubSync(number: 18, after: "sha-own", association: "MEMBER", headRepo: Fork, sender: SenderAuthor, visibility: "public"))).ShouldBe(HttpStatusCode.OK); + + (await LoadRunPayloadsAsync(workflow, "newHeadSha")).ShouldBe(new[] { "sha-own" }, ignoreOrder: true, customMessage: "the outsider's push must start nothing and the member author's own push must run — check the pusher rule in PullRequestTriggerAuthors.AdmitsPusher"); + (await LoadRefusalsAsync(seed)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.AuthorNotMember) && e.Contains("pushed by")); + } + + [Fact] + public async Task A_gitlab_fork_mr_on_a_public_project_runs_only_when_its_author_holds_developer() + { + using var gitlab = new StubProviderHost(); + var projectId = Random.Shared.Next(100_000, 999_999).ToString(); + gitlab.Answer("GET", $"/api/v4/projects/{projectId}/members/all/51", 200, """{"id":51,"username":"dev","access_level":30}""") + .Answer("GET", $"/api/v4/projects/{projectId}/members/all/52", 404, """{"message":"404 Not found"}"""); + var seed = await SeedGitLabRepositoryAsync(gitlab.BaseUrl, projectId); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", "{}"); + + (await PostGitLabAsync(seed, GitLabMr(projectId, iid: 1, authorId: 51))).ShouldBe(HttpStatusCode.OK); + (await PostGitLabAsync(seed, GitLabMr(projectId, iid: 2, authorId: 52))).ShouldBe(HttpStatusCode.OK); + + (await CountRunsAsync(workflow)).ShouldBe(1, customMessage: "the Developer's MR must run and the non-member's must not — check GitLabRepositoryProvider.GetMemberStandingAsync and PullRequestTriggerAdmission"); + (await LoadOnlyRunPayloadAsync(workflow)).GetProperty("authorAssociation").GetString().ShouldBe("member"); + (await LoadRefusalsAsync(seed)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.AuthorNotMember)); + gitlab.Requests.ShouldContain(r => r.PathAndQuery.Contains("/members/all/52"), customMessage: "the standing must come from the provider, not be guessed from the payload"); + } + + // ─── How often ───────────────────────────────────────────────────────────── + + [Fact] + public async Task Closing_and_reopening_a_pr_over_and_over_starts_one_run() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", """{"authors":"any"}"""); + + for (var i = 0; i < 3; i++) + (await PostGitHubAsync(seed, "pull_request", GitHubPr("reopened", number: 11, association: "NONE", headRepo: Fork, visibility: "private", updatedAt: i))).ShouldBe(HttpStatusCode.OK); + + (await CountRunsAsync(workflow)).ShouldBe(1, customMessage: "three reopens inside the debounce window are one run, not three — check PullRequestTriggerAdmission.EnsureNotDebouncedAsync"); + (await LoadRefusalsAsync(seed)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.PullRequestDebounced)); + } + + [Fact] + public async Task A_members_second_push_inside_a_minute_starts_a_run_for_the_commit_it_pushed() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.updated", "{}"); + + await PostGitHubAsync(seed, "pull_request", GitHubSync(number: 19, after: "sha-first", association: "MEMBER", headRepo: SameRepository, sender: SenderAuthor, visibility: "private")); + await PostGitHubAsync(seed, "pull_request", GitHubSync(number: 19, after: "sha-amended", association: "MEMBER", headRepo: SameRepository, sender: SenderAuthor, visibility: "private")); + + (await LoadRunPayloadsAsync(workflow, "newHeadSha")).ShouldBe(new[] { "sha-first", "sha-amended" }, ignoreOrder: true, customMessage: "the commit that ends up on the PR must get a run — the debounce holds off the same head, never newer code"); + } + + [Fact] + public async Task A_delivery_posted_again_after_the_window_does_not_hold_off_the_next_event() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", "{}"); + var opened = GitHubPr("opened", number: 20, association: "MEMBER", headRepo: SameRepository, visibility: "private"); + + (await PostGitHubAsync(seed, "pull_request", opened, deliveryId: "orig-1")).ShouldBe(HttpStatusCode.OK); + await ExpireDebounceAsync(seed); + (await PostGitHubAsync(seed, "pull_request", opened, deliveryId: "orig-1")).ShouldBe(HttpStatusCode.OK); + (await PostGitHubAsync(seed, "pull_request", GitHubPr("reopened", number: 20, association: "MEMBER", headRepo: SameRepository, visibility: "private"))).ShouldBe(HttpStatusCode.OK); + + (await CountRunsAsync(workflow)).ShouldBe(2, customMessage: "the same delivery posted again — Redeliver, or a captured body with its own id — already had its run and must not restart the window; check the already-started check in RunSourceDispatcher.FireIfMatchesAsync"); + (await LoadRefusalsAsync(seed)).ShouldNotContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.PullRequestDebounced)); + } + + [Fact] + public async Task Different_pull_requests_are_not_debounced_against_each_other() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.opened", "{}"); + + await PostGitHubAsync(seed, "pull_request", GitHubPr("opened", number: 12, association: "MEMBER", headRepo: SameRepository, visibility: "private")); + await PostGitHubAsync(seed, "pull_request", GitHubPr("opened", number: 13, association: "MEMBER", headRepo: SameRepository, visibility: "private")); + + (await CountRunsAsync(workflow)).ShouldBe(2); + } + + // ─── Replay ──────────────────────────────────────────────────────────────── + + [Fact] + public async Task A_captured_body_posted_again_under_a_fresh_delivery_id_starts_nothing_and_a_redelivery_stays_one_run() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.merged", "{}"); + var captured = GitHubMerged(number: 14); + + (await PostGitHubAsync(seed, "pull_request", captured, deliveryId: "captured-1")).ShouldBe(HttpStatusCode.OK); + (await PostGitHubAsync(seed, "pull_request", captured, deliveryId: "attacker-2")).ShouldBe(HttpStatusCode.OK); + (await PostGitHubAsync(seed, "pull_request", captured, deliveryId: "captured-1")).ShouldBe(HttpStatusCode.OK); + + (await CountRunsAsync(workflow)).ShouldBe(1, customMessage: "an old merge replayed with a fresh X-GitHub-Delivery must not re-fire a release workflow — check WebhookIngestionService.ClaimBodyOrAuditReplayAsync"); + (await LoadRefusalsAsync(seed)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.DeliveryReplayed)); + } + + [Fact] + public async Task A_signed_github_delivery_without_a_delivery_id_is_refused() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateWorkflowAsync(seed, "trigger.pr.merged", "{}"); + var body = GitHubMerged(number: 15); + + var request = SignedGitHubRequest(seed, "pull_request", body, deliveryId: null); + var response = await _factory.CreateClient().SendAsync(request); + + response.StatusCode.ShouldBe(HttpStatusCode.BadRequest); + (await CountRunsAsync(workflow)).ShouldBe(0, customMessage: "without a delivery id every post of one body reads as a new delivery; GitHub always sends one"); + (await LoadRefusalsAsync(seed)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.DeliveryIdMissing)); + } + + // ─── Callbacks ───────────────────────────────────────────────────────────── + + [Fact] + public async Task A_callback_token_the_engine_minted_resumes_its_run_over_http() + { + // The callback route admits only the minted token's shape; this proves a token the engine really minted fits it. + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + var workflow = await CreateCallbackWorkflowAsync(seed); + await PostGitHubAsync(seed, "pull_request", GitHubMerged(number: 21)); + var runId = await LoadOnlyRunIdAsync(workflow); + + await RunEngineAsync(runId); + var token = await LoadCallbackTokenAsync(runId); + + var response = await _factory.CreateClient().PostAsync($"/api/workflows/callbacks/{token}", new StringContent("""{"status":"done"}""", Encoding.UTF8, "application/json")); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, customMessage: $"the engine minted '{token}' ({token.Length} chars) and the callback route did not take it — the {{token:length(...)}} constraint on WorkflowCallbacksController.Resume must match what WorkflowEngine mints"); + (await response.Content.ReadAsStringAsync()).ShouldContain("\"resumed\":true"); + } + + // ─── What an unauthenticated stranger costs ──────────────────────────────── + + [Fact] + public async Task Forged_signatures_leave_one_bounded_refusal_row_however_many_arrive() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + + for (var i = 0; i < 20; i++) + { + var request = new HttpRequestMessage(HttpMethod.Post, $"/api/webhooks/{seed.WebhookId}") { Content = new StringContent("{}", Encoding.UTF8, "application/json") }; + request.Headers.Add("X-GitHub-Event", "pull_request"); + request.Headers.Add("X-Hub-Signature-256", "sha256=" + new string('f', 64)); + request.Headers.TryAddWithoutValidation("User-Agent", "" + new string('a', 3000)); + request.Headers.TryAddWithoutValidation("X-" + new string('j', 2000) + i, "junk"); + + (await _factory.CreateClient().SendAsync(request)).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + } + + var rows = await LoadRejectedRowsAsync(seed); + rows.Count.ShouldBe(1, customMessage: "twenty forged posts must not push fifty genuine refusals out of the operator's panel — check WebhookIngestionService.BuildRefusalWindowKey on the signature refusal"); + rows[0].Error.ShouldStartWith(WorkflowRunRequestRejectionReasons.SignatureInvalid); + rows[0].RawHeadersRedactedJson!.Length.ShouldBeLessThan(4096, "the stored headers are bounded whatever the stranger posted"); + } + + [Fact] + public async Task A_body_posted_to_an_id_naming_no_hook_is_refused_before_it_is_read() + { + // Larger than the size limit: if the body were read before the hook was resolved, this would be a 413. + var response = await PostRawAsync($"/api/webhooks/{Guid.NewGuid()}", WebhooksController.MaxDeliveryBodyBytes + 1); + var connection = await PostRawAsync($"/api/webhooks/connection/{Guid.NewGuid()}", WebhooksController.MaxDeliveryBodyBytes + 1); + + response.StatusCode.ShouldBe(HttpStatusCode.NotFound); + connection.StatusCode.ShouldBe(HttpStatusCode.NotFound); + } + + [Fact] + public async Task A_body_larger_than_any_provider_sends_is_refused() + { + var seed = await SeedGitHubRepositoryAsync(RepositoryVisibility.Private); + + var response = await PostRawAsync($"/api/webhooks/{seed.WebhookId}", WebhooksController.MaxDeliveryBodyBytes + 1); + + response.StatusCode.ShouldBe(HttpStatusCode.RequestEntityTooLarge); + (await LoadRejectedRowsAsync(seed)).ShouldBeEmpty("an oversized body is refused before anything reads it, so it leaves no row"); + } + + [Fact] + public async Task A_callback_body_larger_than_the_limit_is_refused() + { + var response = await PostRawAsync($"/api/workflows/callbacks/{Guid.NewGuid():N}", WorkflowCallbacksController.MaxCallbackBodyBytes + 1); + + response.StatusCode.ShouldBe(HttpStatusCode.RequestEntityTooLarge); + } + + [Fact] + public async Task One_source_hammering_one_hook_id_is_rate_limited() + { + var target = $"/api/webhooks/{Guid.NewGuid()}"; + var client = _factory.CreateClient(); + + for (var i = 0; i < WebhookIngressRateLimitExtension.PermitsPerWindow; i++) + (await client.PostAsync(target, new StringContent("{}"))).StatusCode.ShouldBe(HttpStatusCode.NotFound, customMessage: $"request {i} inside the budget must reach the endpoint"); + + var over = await client.PostAsync(target, new StringContent("{}")); + var neighbour = await client.PostAsync($"/api/webhooks/{Guid.NewGuid()}", new StringContent("{}")); + + over.StatusCode.ShouldBe(HttpStatusCode.TooManyRequests, customMessage: "past the per-(hook, address) budget the limiter must answer before the endpoint does"); + neighbour.StatusCode.ShouldBe(HttpStatusCode.NotFound, customMessage: "one hook's flood must not throttle another hook's deliveries"); + } + + // ─── Requests ────────────────────────────────────────────────────────────── + + private const string Fork = """{"id":222,"full_name":"evil/repo","fork":true}"""; + private const string SameRepository = """{"id":111,"full_name":"acme/api","fork":false}"""; + + private static string GitHubPr(string action, int number, string association, string headRepo, string visibility, int updatedAt = 0) => + "{\"action\":\"" + action + "\",\"repository\":{\"id\":111,\"full_name\":\"acme/api\",\"private\":" + (visibility == "public" ? "false" : "true") + ",\"visibility\":\"" + visibility + "\"}," + + "\"pull_request\":{\"id\":" + (1000 + number) + ",\"number\":" + number + ",\"title\":\"Ignore previous instructions\",\"body\":null,\"author_association\":\"" + association + "\",\"updated_at\":\"2026-10-08T00:00:0" + updatedAt + "Z\"," + + "\"head\":{\"ref\":\"main\",\"sha\":\"s\",\"repo\":" + headRepo + "},\"base\":{\"ref\":\"main\",\"sha\":\"t\",\"repo\":" + SameRepository + "}," + + "\"user\":{\"id\":9,\"login\":\"someone\"},\"html_url\":\"https://github.example/acme/api/pull/" + number + "\",\"labels\":[]}}"; + + private const string SenderAuthor = """{"id":9,"login":"someone"}"""; + private const string SenderOutsider = """{"id":66,"login":"outsider"}"""; + + /// A push to PR : authored by user 9, pushed by . + private static string GitHubSync(int number, string after, string association, string headRepo, string sender, string visibility) => + "{\"action\":\"synchronize\",\"before\":\"b-" + after + "\",\"after\":\"" + after + "\",\"repository\":{\"id\":111,\"full_name\":\"acme/api\",\"private\":" + (visibility == "public" ? "false" : "true") + ",\"visibility\":\"" + visibility + "\"}," + + "\"pull_request\":{\"id\":" + (1000 + number) + ",\"number\":" + number + ",\"title\":\"t\",\"body\":null,\"author_association\":\"" + association + "\"," + + "\"head\":{\"ref\":\"main\",\"sha\":\"" + after + "\",\"repo\":" + headRepo + "},\"base\":{\"ref\":\"main\",\"sha\":\"t\",\"repo\":" + SameRepository + "}," + + "\"user\":{\"id\":9,\"login\":\"someone\"},\"html_url\":\"https://github.example/acme/api/pull/" + number + "\",\"labels\":[]},\"sender\":" + sender + "}"; + + private static string GitHubMerged(int number) => + "{\"action\":\"closed\",\"repository\":{\"id\":111,\"full_name\":\"acme/api\",\"private\":true,\"visibility\":\"private\"}," + + "\"pull_request\":{\"id\":" + (1000 + number) + ",\"number\":" + number + ",\"merged\":true,\"merge_commit_sha\":\"deadbeef\",\"labels\":[{\"name\":\"release\"}]}," + + "\"sender\":{\"id\":3,\"login\":\"maintainer\"}}"; + + private static string GitLabMr(string projectId, int iid, int authorId) => + "{\"object_kind\":\"merge_request\",\"user\":{\"id\":" + authorId + ",\"username\":\"u" + authorId + "\"}," + + "\"project\":{\"id\":" + projectId + ",\"path_with_namespace\":\"acme/web\",\"visibility_level\":20}," + + "\"object_attributes\":{\"id\":" + (9000 + iid) + ",\"iid\":" + iid + ",\"author_id\":" + authorId + ",\"source_project_id\":777,\"target_project_id\":" + projectId + "," + + "\"source\":{\"path_with_namespace\":\"evil/web\"},\"target\":{\"path_with_namespace\":\"acme/web\"}," + + "\"action\":\"open\",\"title\":\"t\",\"description\":\"d\",\"source_branch\":\"main\",\"target_branch\":\"main\",\"url\":\"https://x\"},\"labels\":[]}"; + + private HttpRequestMessage SignedGitHubRequest(RepositorySeed seed, string eventName, string body, string? deliveryId) + { + var request = new HttpRequestMessage(HttpMethod.Post, $"/api/webhooks/{seed.WebhookId}") { Content = new StringContent(body, Encoding.UTF8, "application/json") }; + request.Headers.Add("X-GitHub-Event", eventName); + if (deliveryId != null) request.Headers.Add("X-GitHub-Delivery", deliveryId); + using var hmac = new HMACSHA256(Encoding.UTF8.GetBytes(seed.Secret)); + request.Headers.Add("X-Hub-Signature-256", "sha256=" + Convert.ToHexString(hmac.ComputeHash(Encoding.UTF8.GetBytes(body))).ToLowerInvariant()); + return request; + } + + private async Task PostGitHubAsync(RepositorySeed seed, string eventName, string body, string? deliveryId = null) => + (await _factory.CreateClient().SendAsync(SignedGitHubRequest(seed, eventName, body, deliveryId ?? $"d-{Guid.NewGuid():N}"))).StatusCode; + + private async Task PostGitLabAsync(RepositorySeed seed, string body) + { + var request = new HttpRequestMessage(HttpMethod.Post, $"/api/webhooks/{seed.WebhookId}") { Content = new StringContent(body, Encoding.UTF8, "application/json") }; + request.Headers.Add("X-Gitlab-Event", "Merge Request Hook"); + request.Headers.Add("X-Gitlab-Event-UUID", $"u-{Guid.NewGuid():N}"); + request.Headers.Add("X-Gitlab-Token", seed.Secret); + return (await _factory.CreateClient().SendAsync(request)).StatusCode; + } + + private async Task PostRawAsync(string url, long bytes) + { + var content = new ByteArrayContent(new byte[bytes]); + content.Headers.ContentType = new MediaTypeHeaderValue("application/json"); + return await _factory.CreateClient().PostAsync(url, content); + } + + private static HttpRequestMessage Authed(HttpMethod method, string url, Guid userId, Guid teamId, string? json = null) + { + var request = new HttpRequestMessage(method, url); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", TestToken.Mint(userId, TestToken.SeedStamp)); + request.Headers.Add("X-Team-Id", teamId.ToString()); + if (json != null) request.Content = new StringContent(json, Encoding.UTF8, "application/json"); + return request; + } + + /// The frontend's own shape: a trigger node whose config IS the activation config, the way deriveActivations projects it. + private async Task CreateWorkflowAsync(RepositorySeed seed, string typeKey, string configJson) + { + var json = "{\"name\":\"outsider-" + Guid.NewGuid().ToString("N")[..6] + "\",\"enabled\":true,\"definition\":{\"schemaVersion\":1" + + ",\"nodes\":[{\"id\":\"trigger\",\"typeKey\":\"" + typeKey + "\",\"label\":\"t\",\"config\":" + configJson + ",\"inputs\":{}},{\"id\":\"end\",\"typeKey\":\"builtin.terminal\",\"label\":\"Done\",\"config\":{},\"inputs\":{}}]" + + ",\"edges\":[{\"from\":\"trigger\",\"to\":\"end\"}]},\"activations\":[{\"typeKey\":\"" + typeKey + "\",\"enabled\":true,\"config\":" + configJson + "}]}"; + + var response = await _factory.CreateClient().SendAsync(Authed(HttpMethod.Post, "/api/workflows", seed.UserId, seed.TeamId, json)); + var body = await response.Content.ReadAsStringAsync(); + response.StatusCode.ShouldBe(HttpStatusCode.OK, customMessage: $"seeding a workflow over POST /api/workflows failed: {body}"); + return JsonDocument.Parse(body).RootElement.GetProperty("id").GetGuid(); + } + + /// A merged PR parks the run on flow.wait_callback until the tokened URL is posted to. + private async Task CreateCallbackWorkflowAsync(RepositorySeed seed) + { + var json = "{\"name\":\"callback-" + Guid.NewGuid().ToString("N")[..6] + "\",\"enabled\":true,\"definition\":{\"schemaVersion\":1" + + ",\"nodes\":[{\"id\":\"trigger\",\"typeKey\":\"trigger.pr.merged\",\"label\":\"t\",\"config\":{},\"inputs\":{}},{\"id\":\"callback\",\"typeKey\":\"flow.wait_callback\",\"label\":\"Wait\",\"config\":{},\"inputs\":{}},{\"id\":\"end\",\"typeKey\":\"builtin.terminal\",\"label\":\"Done\",\"config\":{},\"inputs\":{}}]" + + ",\"edges\":[{\"from\":\"trigger\",\"to\":\"callback\"},{\"from\":\"callback\",\"to\":\"end\"}]},\"activations\":[{\"typeKey\":\"trigger.pr.merged\",\"enabled\":true,\"config\":{}}]}"; + + var response = await _factory.CreateClient().SendAsync(Authed(HttpMethod.Post, "/api/workflows", seed.UserId, seed.TeamId, json)); + var body = await response.Content.ReadAsStringAsync(); + response.StatusCode.ShouldBe(HttpStatusCode.OK, customMessage: $"seeding a workflow over POST /api/workflows failed: {body}"); + return JsonDocument.Parse(body).RootElement.GetProperty("id").GetGuid(); + } + + /// The engine, in-process: the factory's job client is a no-op, so nothing else would ever execute the run. + private async Task RunEngineAsync(Guid runId) + { + using var scope = _factory.Services.CreateScope(); + await scope.ServiceProvider.GetRequiredService().ExecuteRunAsync(runId, CancellationToken.None); + } + + /// The window passing, without waiting it out: every debounce claim on this seed's repository ends a minute ago. + private async Task ExpireDebounceAsync(RepositorySeed seed) + { + using var scope = _factory.Services.CreateScope(); + var pattern = $"pr-debounce:%:{seed.RepositoryId:N}:%"; + await scope.ServiceProvider.GetRequiredService().Database.ExecuteSqlInterpolatedAsync($"UPDATE webhook_claim SET claimed_at = now() - interval '2 minutes', expires_at = now() - interval '1 minute' WHERE claim_key LIKE {pattern}"); + } + + // ─── Seeds ───────────────────────────────────────────────────────────────── + + private sealed record RepositorySeed(Guid UserId, Guid TeamId, Guid RepositoryId, Guid WebhookId, string Secret); + + private Task SeedGitHubRepositoryAsync(RepositoryVisibility visibility) => SeedRepositoryAsync(ProviderKind.GitHub, $"https://gh-{Guid.NewGuid():N}.invalid", $"ext-{Guid.NewGuid():N}", visibility); + + private Task SeedGitLabRepositoryAsync(string baseUrl, string projectId) => SeedRepositoryAsync(ProviderKind.GitLab, baseUrl, projectId, RepositoryVisibility.Public); + + private async Task SeedRepositoryAsync(ProviderKind provider, string baseUrl, string externalId, RepositoryVisibility visibility) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var encryptor = scope.ServiceProvider.GetRequiredService(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + var seed = new RepositorySeed(Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid(), $"sec-{Guid.NewGuid():N}"); + var instanceId = Guid.NewGuid(); + var credentialId = Guid.NewGuid(); + + db.User.Add(new User { Id = seed.UserId, SecurityStamp = TestToken.SeedStamp, Email = $"owner-{suffix}@test.local", Name = "Owner", CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.Team.Add(new Team { Id = seed.TeamId, Slug = $"team-{suffix}", Name = "Team", Kind = TeamKind.Workspace, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = seed.TeamId, UserId = seed.UserId, Role = TeamRole.Owner, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.ProviderInstance.Add(new ProviderInstance { Id = instanceId, TeamId = seed.TeamId, Provider = provider, DisplayName = "P", BaseUrl = baseUrl, ApiUrl = baseUrl, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.Credential.Add(new Credential { Id = credentialId, TeamId = seed.TeamId, ProviderInstanceId = instanceId, AuthType = AuthType.Pat, DisplayName = "PAT", EncryptedPayload = encryptor.Encrypt("{\"token\":\"fake-token\"}"), CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.Repository.Add(new Repository { Id = seed.RepositoryId, TeamId = seed.TeamId, ProviderInstanceId = instanceId, CredentialId = credentialId, ExternalId = externalId, NamespacePath = "acme", Name = "api", FullPath = $"acme/api-{suffix}", Visibility = visibility, WebUrl = "https://p.invalid/acme/api", CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.RepositoryWebhook.Add(new RepositoryWebhook { Id = seed.WebhookId, RepositoryId = seed.RepositoryId, ExternalId = "wh-1", CallbackUrl = $"https://x/cb/{suffix}", SecretEnc = encryptor.Encrypt(seed.Secret), Active = true, SubscribedEvents = new List { "*" }, RegistrationStatus = RepositoryWebhookRegistrationStatus.Registered, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + + await db.SaveChangesAsync(); + return seed; + } + + // ─── Reads ───────────────────────────────────────────────────────────────── + + private async Task CountRunsAsync(Guid workflowId) + { + using var scope = _factory.Services.CreateScope(); + return await scope.ServiceProvider.GetRequiredService().WorkflowRun.AsNoTracking().CountAsync(r => r.WorkflowId == workflowId); + } + + private async Task LoadOnlyRunPayloadAsync(Guid workflowId) + { + using var scope = _factory.Services.CreateScope(); + var payloads = await scope.ServiceProvider.GetRequiredService().WorkflowRun.AsNoTracking() + .Where(r => r.WorkflowId == workflowId) + .Select(r => r.RunRequest.NormalizedPayloadJson) + .ToListAsync(); + + payloads.Count.ShouldBe(1, customMessage: "expected exactly one run for this workflow"); + return JsonDocument.Parse(payloads[0]).RootElement; + } + + private async Task> LoadRunPayloadsAsync(Guid workflowId, string field) + { + using var scope = _factory.Services.CreateScope(); + var payloads = await scope.ServiceProvider.GetRequiredService().WorkflowRun.AsNoTracking().Where(r => r.WorkflowId == workflowId).Select(r => r.RunRequest.NormalizedPayloadJson).ToListAsync(); + return payloads.Select(p => JsonDocument.Parse(p).RootElement.GetProperty(field).GetString()!).ToList(); + } + + private async Task LoadOnlyRunIdAsync(Guid workflowId) + { + using var scope = _factory.Services.CreateScope(); + return await scope.ServiceProvider.GetRequiredService().WorkflowRun.AsNoTracking().Where(r => r.WorkflowId == workflowId).Select(r => r.Id).SingleAsync(); + } + + private async Task LoadCallbackTokenAsync(Guid runId) + { + using var scope = _factory.Services.CreateScope(); + return await scope.ServiceProvider.GetRequiredService().WorkflowRunWait.AsNoTracking().Where(w => w.RunId == runId && w.WaitKind == WorkflowWaitKinds.Callback).Select(w => w.Token).SingleAsync(); + } + + private async Task> LoadRefusalsAsync(RepositorySeed seed) => (await LoadRejectedRowsAsync(seed)).Select(r => r.Error!).ToList(); + + private async Task> LoadRejectedRowsAsync(RepositorySeed seed) + { + using var scope = _factory.Services.CreateScope(); + return await scope.ServiceProvider.GetRequiredService().WorkflowRunRequest.AsNoTracking() + .Where(r => r.TeamId == seed.TeamId && r.Status == WorkflowRunRequestStatus.Rejected && r.Error != null && !r.Error.StartsWith(WorkflowRunRequestRejectionReasons.NoMatchingActivation)) + .ToListAsync(); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookTenancyE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookTenancyE2ETests.cs index d031b0efa..1fdb76dd9 100644 --- a/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookTenancyE2ETests.cs +++ b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookTenancyE2ETests.cs @@ -205,8 +205,8 @@ public async Task An_owner_rename_followed_by_a_delivery_still_starts_exactly_on var newHook = await LoadConnectionHookAsync(seed.InstanceId, renamed); newHook.ShouldNotBeNull(customMessage: "the refresh moved the repository out from under every hook and staged none. Check RepositoryService.RefreshMetadataBestEffortAsync stages coverage through IConnectionWebhookCoverageService."); - // From here the provider sends each event on both of the renamed group's hooks. - var afterRename = GitLabOpenedBody(projectId, $"{renamed}/web"); + // From here the provider sends each event on both of the renamed group's hooks — here a new MR opened after the rename. + var afterRename = GitLabOpenedBody(projectId, $"{renamed}/web", iid: 4); (await PostGitLabAsync(oldHook.Url, afterRename, oldHook.Token)).ShouldBe(HttpStatusCode.OK); (await PostGitLabAsync(newHook!.Url, afterRename, newHook.Token)).ShouldBe(HttpStatusCode.OK); @@ -325,8 +325,9 @@ private static string GitHubPushBody(string marker) => private static string GitLabMergeRequestBody() => "{\"object_kind\":\"merge_request\",\"user\":{\"id\":1,\"username\":\"u\"},\"project\":{\"id\":1},\"object_attributes\":{\"id\":1,\"iid\":3,\"action\":\"open\",\"title\":\"victim\",\"description\":\"d\",\"source_branch\":\"f\",\"target_branch\":\"main\",\"url\":\"https://x\"},\"labels\":[]}"; - private static string GitLabOpenedBody(string projectId, string path) => - "{\"object_kind\":\"merge_request\",\"user\":{\"id\":1,\"username\":\"u\"},\"project\":{\"id\":" + projectId + ",\"path_with_namespace\":\"" + path + "\"},\"object_attributes\":{\"id\":1,\"iid\":3,\"action\":\"open\",\"title\":\"t\",\"description\":\"d\",\"source_branch\":\"f\",\"target_branch\":\"main\",\"url\":\"https://x\"},\"labels\":[]}"; + /// A merge request opened on the project. Each MR its own : the same MR opened twice inside the debounce window is one run. + private static string GitLabOpenedBody(string projectId, string path, int iid = 3) => + "{\"object_kind\":\"merge_request\",\"user\":{\"id\":1,\"username\":\"u\"},\"project\":{\"id\":" + projectId + ",\"path_with_namespace\":\"" + path + "\"},\"object_attributes\":{\"id\":" + iid + ",\"iid\":" + iid + ",\"action\":\"open\",\"title\":\"t\",\"description\":\"d\",\"source_branch\":\"f\",\"target_branch\":\"main\",\"url\":\"https://x\"},\"labels\":[]}"; /// What GitLab answers for GET /api/v4/projects/:id — trimmed to the fields the provider class maps. private static string GitLabProjectJson(string projectId, string owner) => diff --git a/backend/tests/CodeSpace.IntegrationTests/Webhooks/ConnectionWebhookIngestionTests.cs b/backend/tests/CodeSpace.IntegrationTests/Webhooks/ConnectionWebhookIngestionTests.cs index 568e9c744..0c0fa19b0 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Webhooks/ConnectionWebhookIngestionTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Webhooks/ConnectionWebhookIngestionTests.cs @@ -69,6 +69,53 @@ public async Task GitHub_organization_delivery_routes_to_the_repository_the_payl captured.ShouldNotContain(e => e.RepositoryId == seed.FirstRepositoryId); } + [Fact] + public async Task A_github_organization_delivery_without_a_delivery_id_is_refused_and_starts_nothing() + { + var secret = $"gh-conn-{Guid.NewGuid():N}"; + var seed = await SeedConnectionAsync(ProviderKind.GitHub, secret).ConfigureAwait(false); + var body = BuildGitHubPushBody(repositoryId: seed.SecondExternalId, fullName: seed.SecondFullPath); + var headers = GitHubHeaders(body, secret); + headers.Remove("X-GitHub-Delivery"); + ClearCapturedEvents(); + + await Should.ThrowAsync(() => IngestAsync(seed.ConnectionWebhookId, body, headers)).ConfigureAwait(false); + + SnapshotCapturedEvents().ShouldBeEmpty(); + (await LoadErrorsForHookAsync(seed.ConnectionWebhookId).ConfigureAwait(false)).ShouldContain(e => e.StartsWith(WorkflowRunRequestRejectionReasons.DeliveryIdMissing)); + } + + [Fact] + public async Task A_github_organization_body_replayed_under_a_fresh_delivery_id_is_not_published_again() + { + var secret = $"gh-conn-{Guid.NewGuid():N}"; + var seed = await SeedConnectionAsync(ProviderKind.GitHub, secret).ConfigureAwait(false); + var body = BuildGitHubPushBody(repositoryId: seed.SecondExternalId, fullName: seed.SecondFullPath); + ClearCapturedEvents(); + + await IngestAsync(seed.ConnectionWebhookId, body, GitHubHeaders(body, secret, "org-captured-1")).ConfigureAwait(false); + await IngestAsync(seed.ConnectionWebhookId, body, GitHubHeaders(body, secret, "org-attacker-2")).ConfigureAwait(false); + + SnapshotCapturedEvents().OfType().Select(e => e.ProviderEventId).ShouldBe(new[] { "org-captured-1" }); + (await CountRefusalsAsync(seed.SecondRepositoryId, WorkflowRunRequestRejectionReasons.DeliveryReplayed).ConfigureAwait(false)).ShouldBe(1); + } + + [Fact] + public async Task A_gitlab_group_body_posted_twice_is_published_twice_because_its_token_already_signs_anything() + { + // GitLab authenticates with a static token header, so whoever holds it can sign any body — a replay check would + // stop nothing the token does not already allow, and an older GitLab sends no delivery id to hold a claim with. + var secret = $"gl-conn-{Guid.NewGuid():N}"; + var seed = await SeedConnectionAsync(ProviderKind.GitLab, secret).ConfigureAwait(false); + var body = BuildGitLabPushBody(projectId: seed.SecondExternalId, path: seed.SecondFullPath); + ClearCapturedEvents(); + + await IngestAsync(seed.ConnectionWebhookId, body, GitLabHeaders(secret)).ConfigureAwait(false); + await IngestAsync(seed.ConnectionWebhookId, body, GitLabHeaders(secret)).ConfigureAwait(false); + + SnapshotCapturedEvents().OfType().Count().ShouldBe(2); + } + [Fact] public async Task A_delivery_for_an_unbound_repository_is_dropped_and_audited() { diff --git a/backend/tests/CodeSpace.IntegrationTests/Webhooks/ReceiveWebhookFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Webhooks/ReceiveWebhookFlowTests.cs index e49589116..73755b233 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Webhooks/ReceiveWebhookFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Webhooks/ReceiveWebhookFlowTests.cs @@ -381,6 +381,7 @@ public async Task Unhandled_event_type_returns_without_publish_but_updates_last_ var headers = new Dictionary { ["X-GitHub-Event"] = "ping", + ["X-GitHub-Delivery"] = $"gh-ping-{Guid.NewGuid():N}", ["X-Hub-Signature-256"] = ComputeGitHubSignature(body, secret) }; @@ -401,6 +402,7 @@ public async Task GitHub_push_event_publishes_PushReceivedEvent() var headers = new Dictionary { ["X-GitHub-Event"] = "push", + ["X-GitHub-Delivery"] = $"gh-push-{Guid.NewGuid():N}", ["X-Hub-Signature-256"] = ComputeGitHubSignature(body, secret) }; diff --git a/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookClaimStoreTests.cs b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookClaimStoreTests.cs new file mode 100644 index 000000000..7b861fb54 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookClaimStoreTests.cs @@ -0,0 +1,181 @@ +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.Webhooks; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Commands.Webhooks; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Webhooks; + +/// +/// The claim the replay check and the PR debounce both stand on, against real Postgres: one holder per key per window, +/// the same holder re-claiming freely (a provider redelivery), an expired claim taken over in place, two concurrent +/// claimants settled by the primary key rather than by a read both could pass, and a rolled-back delivery releasing its +/// claim so the provider's retry is not refused. Expired rows are cleared by the recurring sweep, outside any delivery's +/// transaction — a delivery that cleared them itself held their row locks until it committed, and two of them could +/// each wait on a key the other had just deleted. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class WebhookClaimStoreTests +{ + private static readonly TimeSpan Window = TimeSpan.FromMinutes(5); + + private readonly PostgresFixture _fixture; + + public WebhookClaimStoreTests(PostgresFixture fixture) { _fixture = fixture; } + + [Fact] + public async Task A_key_is_held_by_its_first_holder_and_free_to_that_holder_again() + { + var key = NewKey(); + + (await ClaimAsync(key, "delivery-1")).ShouldBeTrue("a free key"); + (await ClaimAsync(key, "delivery-2")).ShouldBeFalse("held by another holder inside its window"); + (await ClaimAsync(key, "delivery-1")).ShouldBeTrue("the same holder — a provider redelivering the same delivery — is not a second event"); + } + + [Fact] + public async Task An_expired_claim_is_taken_over_by_the_next_holder() + { + var key = NewKey(); + await ClaimAsync(key, "delivery-1"); + await ExpireAsync(key); + + (await ClaimAsync(key, "delivery-2")).ShouldBeTrue(); + (await LoadHolderAsync(key)).ShouldBe("delivery-2"); + } + + [Fact] + public async Task Concurrent_claimants_are_settled_by_the_key_and_exactly_one_wins() + { + var key = NewKey(); + using var firstScope = _fixture.BeginScope(); + using var secondScope = _fixture.BeginScope(); + var firstDb = firstScope.Resolve(); + var secondDb = secondScope.Resolve(); + + await using var first = await firstDb.Database.BeginTransactionAsync(); + await using var second = await secondDb.Database.BeginTransactionAsync(); + + (await firstScope.Resolve().TryClaimAsync(key, "delivery-1", Window, CancellationToken.None)).ShouldBeTrue(); + + var contender = secondScope.Resolve().TryClaimAsync(key, "delivery-2", Window, CancellationToken.None); + await Task.Delay(200); + contender.IsCompleted.ShouldBeFalse("the second claimant must wait on the first's uncommitted row, not read past it"); + + await first.CommitAsync(); + + (await contender.WaitAsync(TimeSpan.FromSeconds(10))).ShouldBeFalse(customMessage: "once the first commits, the second sees its claim — check the ON CONFLICT ... WHERE in WebhookClaimStore.UpsertClaimAsync"); + await second.CommitAsync(); + } + + [Fact] + public async Task A_rolled_back_delivery_releases_its_claim() + { + var key = NewKey(); + + using (var scope = _fixture.BeginScope()) + { + await using var transaction = await scope.Resolve().Database.BeginTransactionAsync(); + (await scope.Resolve().TryClaimAsync(key, "delivery-1", Window, CancellationToken.None)).ShouldBeTrue(); + await transaction.RollbackAsync(); + } + + (await ClaimAsync(key, "delivery-2")).ShouldBeTrue("a delivery that failed and rolled back must not refuse the provider's retry of the same event"); + } + + [Fact] + public async Task Two_deliveries_claiming_among_many_expired_keys_do_not_deadlock() + { + // The interleaving that deadlocked when each claim purged a batch of expired rows inside its own delivery's + // transaction: each delivery's purge locked (deleted) a different batch, then each upserted a key sitting in the + // OTHER's batch, and each waited on the other's commit. + var prefix = $"test:{Guid.NewGuid():N}:"; + await SeedExpiredAsync(prefix, count: 150); + + using var firstScope = _fixture.BeginScope(); + using var secondScope = _fixture.BeginScope(); + await using var first = await firstScope.Resolve().Database.BeginTransactionAsync(); + await using var second = await secondScope.Resolve().Database.BeginTransactionAsync(); + var firstStore = firstScope.Resolve(); + var secondStore = secondScope.Resolve(); + + (await firstStore.TryClaimAsync(NewKey(), "delivery-a", Window, CancellationToken.None)).ShouldBeTrue(); + (await secondStore.TryClaimAsync(NewKey(), "delivery-b", Window, CancellationToken.None)).ShouldBeTrue(); + + var firstTakesOver = firstStore.TryClaimAsync(prefix + 120, "delivery-a", Window, CancellationToken.None); + var secondTakesOver = secondStore.TryClaimAsync(prefix + 50, "delivery-b", Window, CancellationToken.None); + + (await firstTakesOver.WaitAsync(TimeSpan.FromSeconds(10))).ShouldBeTrue(customMessage: "an expired key is taken over in place without waiting on anyone — a wait here means a claim is locking rows it did not ask for"); + (await secondTakesOver.WaitAsync(TimeSpan.FromSeconds(10))).ShouldBeTrue(); + + await first.CommitAsync(); + await second.CommitAsync(); + } + + [Fact] + public async Task The_sweep_deletes_expired_claims_and_keeps_live_ones() + { + var stale = NewKey(); + var live = NewKey(); + await ClaimAsync(stale, "old"); + await ClaimAsync(live, "new"); + await ExpireAsync(stale, at: new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero)); + + using (var scope = _fixture.BeginScope()) + (await scope.Resolve().Send(new PurgeExpiredWebhookClaimsCommand())).Deleted.ShouldBeGreaterThanOrEqualTo(1); + + (await LoadHolderAsync(stale)).ShouldBeNull("the sweep is what keeps the table to about one window of deliveries"); + (await LoadHolderAsync(live)).ShouldBe("new", "a claim inside its window is still holding something off"); + } + + [Fact] + public async Task The_sweep_steps_around_a_claim_an_open_delivery_holds_instead_of_waiting_on_it() + { + var held = NewKey(); + await ClaimAsync(held, "old"); + await ExpireAsync(held); + + using var deliveryScope = _fixture.BeginScope(); + await using var delivery = await deliveryScope.Resolve().Database.BeginTransactionAsync(); + (await deliveryScope.Resolve().TryClaimAsync(held, "delivery-1", Window, CancellationToken.None)).ShouldBeTrue("the delivery takes the expired key over and holds its row until it commits"); + + using (var sweepScope = _fixture.BeginScope()) + await sweepScope.Resolve().Send(new PurgeExpiredWebhookClaimsCommand()).WaitAsync(TimeSpan.FromSeconds(10)); + + await delivery.CommitAsync(); + + (await LoadHolderAsync(held)).ShouldBe("delivery-1", "the sweep must skip a row a delivery has locked — waiting on it is how a sweep joins a deadlock, and deleting it would drop a live claim"); + } + + private static string NewKey() => $"test:{Guid.NewGuid():N}"; + + /// claims under , expired in key order — the oldest first, as a purge batch would pick them. + private async Task SeedExpiredAsync(string prefix, int count) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"INSERT INTO webhook_claim SELECT {prefix} || g, 'old-' || g, now() - interval '1 hour', now() - make_interval(secs => 300 - g) FROM generate_series(0, {count - 1}) g"); + } + + private async Task ClaimAsync(string key, string holder) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().TryClaimAsync(key, holder, Window, CancellationToken.None); + } + + private async Task ExpireAsync(string key, DateTimeOffset? at = null) + { + using var scope = _fixture.BeginScope(); + var expiry = at ?? DateTimeOffset.UtcNow.AddMinutes(-1); + await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE webhook_claim SET claimed_at = {expiry}, expires_at = {expiry} WHERE claim_key = {key}"); + } + + private async Task LoadHolderAsync(string key) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().Database.SqlQuery($"SELECT holder AS \"Value\" FROM webhook_claim WHERE claim_key = {key}").SingleOrDefaultAsync(); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressContractTests.cs b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressContractTests.cs new file mode 100644 index 000000000..2f6c4e3fd --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressContractTests.cs @@ -0,0 +1,88 @@ +using System.Net; +using System.Reflection; +using CodeSpace.Api.Controllers; +using CodeSpace.Api.Extensions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Metadata; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.RateLimiting; +using Microsoft.AspNetCore.Routing; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Webhooks; + +/// +/// The bounds on the anonymous ingress actions, pinned where a refactor would quietly drop them: every webhook and +/// callback action carries its body limit and the ingress rate-limit policy, every route value the policy keys on is +/// bounded in length by its route, and the limits partition first by who sent the request and then by what it is +/// addressed to. The behaviour behind each is proven over HTTP in WebhookOutsiderHardeningE2ETests and +/// WebhookIngressSourceLimitE2ETests. +/// +[Trait("Category", "Integration")] +public sealed class WebhookIngressContractTests +{ + [Theory] + [InlineData(typeof(WebhooksController), nameof(WebhooksController.Receive), WebhooksController.MaxDeliveryBodyBytes)] + [InlineData(typeof(WebhooksController), nameof(WebhooksController.ReceiveConnection), WebhooksController.MaxDeliveryBodyBytes)] + [InlineData(typeof(WorkflowCallbacksController), nameof(WorkflowCallbacksController.Resume), WorkflowCallbacksController.MaxCallbackBodyBytes)] + public void Every_anonymous_ingress_action_bounds_its_body_and_is_rate_limited(Type controller, string action, long maxBytes) + { + var method = controller.GetMethod(action, BindingFlags.Instance | BindingFlags.Public).ShouldNotBeNull(); + + ((IRequestSizeLimitMetadata)method.GetCustomAttribute().ShouldNotBeNull()).MaxRequestBodySize.ShouldBe(maxBytes); + controller.GetCustomAttribute().ShouldNotBeNull().PolicyName.ShouldBe(WebhookIngressRateLimitExtension.PolicyName); + } + + [Fact] + public void The_limits_are_pinned() + { + // GitHub caps a payload at 25 MB; a callback is a resume signal. The ceiling sits above a busy group hook's traffic. + WebhooksController.MaxDeliveryBodyBytes.ShouldBe(25L * 1024 * 1024); + WorkflowCallbacksController.MaxCallbackBodyBytes.ShouldBe(1024 * 1024); + WebhookIngressRateLimitExtension.PolicyName.ShouldBe("webhook-ingress"); + WebhookIngressRateLimitExtension.PermitsPerWindow.ShouldBe(2000); + WebhookIngressRateLimitExtension.SourcePermitsPerWindow.ShouldBe(10_000); + WebhookIngressRateLimitExtension.Window.ShouldBe(TimeSpan.FromMinutes(1)); + } + + [Theory] + [InlineData(typeof(WebhooksController), nameof(WebhooksController.Receive), "{webhookId:guid}")] + [InlineData(typeof(WebhooksController), nameof(WebhooksController.ReceiveConnection), "connection/{connectionWebhookId:guid}")] + [InlineData(typeof(WorkflowCallbacksController), nameof(WorkflowCallbacksController.Resume), "{token:length(32)}")] + public void Every_route_value_the_limiter_keys_on_is_bounded_by_its_route(Type controller, string action, string template) + { + // The per-target limiter keys on these values before anything is looked up. Unconstrained, a token could be as + // long as the request line, and every distinct one kept a limiter of that size alive past its window. 32 is the + // engine's minted callback token: Guid.NewGuid().ToString("N"). + var method = controller.GetMethod(action, BindingFlags.Instance | BindingFlags.Public).ShouldNotBeNull(); + + method.GetCustomAttribute().ShouldNotBeNull().Template.ShouldBe(template); + } + + [Theory] + [InlineData("203.0.113.7", "203.0.113.7")] + [InlineData(null, "unknown")] + public void The_source_partition_is_who_sent_the_request_and_nothing_they_chose(string? address, string expected) + { + var context = new DefaultHttpContext(); + context.Request.RouteValues = new RouteValueDictionary { ["token"] = new string('a', 32) }; + context.Connection.RemoteIpAddress = address == null ? null : IPAddress.Parse(address); + + WebhookIngressRateLimitExtension.SourcePartitionKey(context).ShouldBe(expected); + } + + [Theory] + [InlineData("webhookId", "3f2c", "203.0.113.7", "3f2c|203.0.113.7")] + [InlineData("connectionWebhookId", "9a1b", "203.0.113.7", "9a1b|203.0.113.7")] + [InlineData("token", "tok", "198.51.100.2", "tok|198.51.100.2")] + [InlineData("token", "tok", null, "tok|unknown")] + [InlineData("other", "x", "198.51.100.2", "-|198.51.100.2")] + public void A_partition_is_what_the_request_is_addressed_to_and_who_sent_it(string routeValue, string target, string? address, string expected) + { + var context = new DefaultHttpContext(); + context.Request.RouteValues = new RouteValueDictionary { [routeValue] = target }; + context.Connection.RemoteIpAddress = address == null ? null : IPAddress.Parse(address); + + WebhookIngressRateLimitExtension.PartitionKey(context).ShouldBe(expected); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressRefusalFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressRefusalFlowTests.cs new file mode 100644 index 000000000..c1f3e1ff9 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookIngressRefusalFlowTests.cs @@ -0,0 +1,188 @@ +using System.Security.Cryptography; +using System.Text; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Webhooks; +using CodeSpace.Core.Services.Webhooks.Exceptions; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Commands.Webhooks; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Events; +using CodeSpace.Messages.Events.PullRequest; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Webhooks; + +/// +/// What a per-repository hook keeps when a stranger or a replay reaches it, through the real command, transaction, +/// auditor and claim table: refusals anyone can cause collapse to one bounded row per (hook, reason) per day; a signed +/// GitHub body without its delivery id is refused; a signed body replayed under a fresh id is not published again, +/// while the provider redelivering the SAME id still is (the per-activation idempotency key handles that one). +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class WebhookIngressRefusalFlowTests +{ + private readonly PostgresFixture _fixture; + + public WebhookIngressRefusalFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Fact] + public async Task Bad_signatures_collapse_to_one_bounded_row_and_keep_the_providers_own_headers() + { + var seed = await SeedAsync(ProviderKind.GitHub); + + for (var i = 0; i < 5; i++) + { + var headers = new Dictionary { ["X-GitHub-Event"] = "pull_request", ["X-GitHub-Delivery"] = $"forged-{i}", ["X-Hub-Signature-256"] = "sha256=00", ["User-Agent"] = new string('<', 4000) }; + foreach (var junk in Enumerable.Range(0, 100)) headers[$"X-Junk-{junk}-{new string('n', 300)}"] = "v"; + + await Should.ThrowAsync(() => SendAsync(seed.WebhookId, "{}", headers)); + } + + var rows = await LoadRefusalsAsync(seed.RepositoryId, WorkflowRunRequestRejectionReasons.SignatureInvalid); + rows.Count.ShouldBe(1, customMessage: "a refusal anyone can cause is one row per (hook, reason) per day — check BuildRefusalWindowKey in VerifySignatureOrAuditAsync"); + var stored = rows[0].RawHeadersRedactedJson.ShouldNotBeNull(); + stored.Length.ShouldBeLessThan(4096); + stored.ShouldContain("forged-0", customMessage: "the provider's delivery id survives the cap, so the row can be matched against the provider's log"); + } + + [Theory] + [InlineData(HookState.Inactive, WorkflowRunRequestRejectionReasons.WebhookInactive)] + [InlineData(HookState.Retired, WorkflowRunRequestRejectionReasons.WebhookRetired)] + [InlineData(HookState.RepositoryRemoved, WorkflowRunRequestRejectionReasons.WebhookRetired)] + public async Task A_switched_off_or_retired_hook_records_its_refusal_once_however_often_it_is_posted_to(HookState state, string reason) + { + var seed = await SeedAsync(ProviderKind.GitHub); + await PutInStateAsync(seed, state); + + for (var i = 0; i < 3; i++) + await Should.ThrowAsync(() => SendAsync(seed.WebhookId, "{}", new Dictionary { ["X-GitHub-Event"] = "push" })); + + (await LoadRefusalsAsync(seed.RepositoryId, reason)).Count.ShouldBe(1); + } + + [Fact] + public async Task A_signed_github_delivery_without_its_delivery_id_is_refused_and_recorded_once() + { + var seed = await SeedAsync(ProviderKind.GitHub); + var body = OpenedBody(number: 1); + + for (var i = 0; i < 2; i++) + await Should.ThrowAsync(() => SendAsync(seed.WebhookId, body, GitHubHeaders(body, seed.Secret, deliveryId: null))); + + (await LoadRefusalsAsync(seed.RepositoryId, WorkflowRunRequestRejectionReasons.DeliveryIdMissing)).Count.ShouldBe(1); + (await LoadLastReceivedAsync(seed.WebhookId)).ShouldBeNull("a refused delivery does not count as received"); + } + + [Fact] + public async Task A_replayed_body_is_published_once_and_a_redelivery_of_the_same_id_is_published_again() + { + var seed = await SeedAsync(ProviderKind.GitHub); + var body = OpenedBody(number: 2); + ClearCapturedEvents(); + + await SendAsync(seed.WebhookId, body, GitHubHeaders(body, seed.Secret, "captured-1")); + for (var i = 0; i < 3; i++) await SendAsync(seed.WebhookId, body, GitHubHeaders(body, seed.Secret, $"attacker-{i}")); + await SendAsync(seed.WebhookId, body, GitHubHeaders(body, seed.Secret, "captured-1")); + + SnapshotCapturedEvents().OfType().Select(e => e.ProviderEventId).ShouldBe(new[] { "captured-1", "captured-1" }, + customMessage: "a fresh id on an old body is a replay; the same id is the provider redelivering, which must still reach dispatch so an operator's Redeliver works"); + (await LoadRefusalsAsync(seed.RepositoryId, WorkflowRunRequestRejectionReasons.DeliveryReplayed)).Count.ShouldBe(1, customMessage: "one row per replayed body, however often it is posted"); + } + + [Fact] + public async Task A_gitlab_body_posted_twice_is_published_twice() + { + var seed = await SeedAsync(ProviderKind.GitLab); + const string body = """{"object_kind":"merge_request","user":{"id":1,"username":"alice"},"object_attributes":{"id":99,"iid":5,"title":"t","source_branch":"f","target_branch":"main","action":"open","url":"https://x"}}"""; + ClearCapturedEvents(); + + for (var i = 0; i < 2; i++) + await SendAsync(seed.WebhookId, body, new Dictionary { ["X-Gitlab-Event"] = "Merge Request Hook", ["X-Gitlab-Event-UUID"] = $"gl-{Guid.NewGuid():N}", ["X-Gitlab-Token"] = seed.Secret }); + + SnapshotCapturedEvents().OfType().Count().ShouldBe(2, customMessage: "GitLab's token signs any body, so a replay check would stop nothing — it is GitHub-shaped providers only"); + } + + public enum HookState { Inactive, Retired, RepositoryRemoved } + + private static string OpenedBody(int number) => + "{\"action\":\"opened\",\"pull_request\":{\"id\":" + (500 + number) + ",\"number\":" + number + ",\"title\":\"t\",\"head\":{\"ref\":\"f\",\"sha\":\"s\"},\"base\":{\"ref\":\"main\"},\"user\":{\"id\":5,\"login\":\"u\"},\"html_url\":\"https://x\"}}"; + + private static Dictionary GitHubHeaders(string body, string secret, string? deliveryId) + { + using var hmac = new HMACSHA256(Encoding.UTF8.GetBytes(secret)); + var headers = new Dictionary { ["X-GitHub-Event"] = "pull_request", ["X-Hub-Signature-256"] = "sha256=" + Convert.ToHexString(hmac.ComputeHash(Encoding.UTF8.GetBytes(body))).ToLowerInvariant() }; + if (deliveryId != null) headers["X-GitHub-Delivery"] = deliveryId; + return headers; + } + + private async Task SendAsync(Guid webhookId, string body, IReadOnlyDictionary headers) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().Send(new ReceiveWebhookCommand { WebhookId = webhookId, Body = body, Headers = headers }); + } + + private sealed record Seed(Guid WebhookId, Guid RepositoryId, string Secret); + + private async Task SeedAsync(ProviderKind provider) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var encryptor = scope.Resolve(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + var team = new Team { Id = Guid.NewGuid(), Slug = $"ingress-{suffix}", Name = "Team" }; + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = provider, DisplayName = "Inst", BaseUrl = $"https://{provider}-{suffix}.invalid" }; + var repository = new Repository { Id = Guid.NewGuid(), TeamId = team.Id, ProviderInstanceId = instance.Id, ExternalId = $"ext-{suffix}", NamespacePath = "n", Name = "r", FullPath = $"n/r-{suffix}", WebUrl = "https://x" }; + var seed = new Seed(Guid.NewGuid(), repository.Id, $"sec-{Guid.NewGuid():N}"); + + db.Team.Add(team); + db.ProviderInstance.Add(instance); + db.Repository.Add(repository); + db.RepositoryWebhook.Add(new RepositoryWebhook { Id = seed.WebhookId, RepositoryId = repository.Id, ExternalId = $"wh-{suffix}", CallbackUrl = "https://x/cb", SecretEnc = encryptor.Encrypt(seed.Secret), SubscribedEvents = new List { "*" } }); + await db.SaveChangesAsync(); + + return seed; + } + + private async Task PutInStateAsync(Seed seed, HookState state) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + if (state == HookState.Inactive) await db.RepositoryWebhook.Where(w => w.Id == seed.WebhookId).ExecuteUpdateAsync(s => s.SetProperty(w => w.Active, false)); + if (state == HookState.Retired) await db.RepositoryWebhook.Where(w => w.Id == seed.WebhookId).ExecuteUpdateAsync(s => s.SetProperty(w => w.RegistrationStatus, RepositoryWebhookRegistrationStatus.Cancelled)); + if (state == HookState.RepositoryRemoved) await db.Repository.Where(r => r.Id == seed.RepositoryId).ExecuteUpdateAsync(s => s.SetProperty(r => r.DeletedDate, (DateTimeOffset?)DateTimeOffset.UtcNow)); + } + + private async Task> LoadRefusalsAsync(Guid repositoryId, string reason) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunRequest.AsNoTracking() + .Where(r => r.RepositoryId == repositoryId && r.Status == WorkflowRunRequestStatus.Rejected && r.Error!.StartsWith(reason + ":")) + .ToListAsync(); + } + + private async Task LoadLastReceivedAsync(Guid webhookId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().RepositoryWebhook.AsNoTracking().Where(w => w.Id == webhookId).Select(w => w.LastReceivedDate).SingleAsync(); + } + + private void ClearCapturedEvents() + { + using var scope = _fixture.BeginScope(); + scope.Resolve().Clear(); + } + + private IReadOnlyList SnapshotCapturedEvents() + { + using var scope = _fixture.BeginScope(); + return scope.Resolve().Snapshot(); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/PullRequestTriggerAdmissionFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/PullRequestTriggerAdmissionFlowTests.cs new file mode 100644 index 000000000..8ff5b66c8 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/PullRequestTriggerAdmissionFlowTests.cs @@ -0,0 +1,412 @@ +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Commands.Workflows; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Dtos.Workflows; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Events; +using CodeSpace.Messages.Events.PullRequest; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// Who may start a run with a pull request, and how often — through the real RunSourceDispatcher, +/// PullRequestTriggerAdmission, claim table, auditor and RunStarter against Postgres. The GitLab cases ask +/// the real GitLab provider class for the author's standing over the wire, from a loopback . +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class PullRequestTriggerAdmissionFlowTests +{ + private const string Opened = "trigger.pr.opened"; + private const string Updated = "trigger.pr.updated"; + + private readonly PostgresFixture _fixture; + + public PullRequestTriggerAdmissionFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + // ─── Authors ─────────────────────────────────────────────────────────────── + + [Theory] + [InlineData(Opened, RepositoryVisibility.Public)] + [InlineData(Updated, RepositoryVisibility.Public)] + [InlineData(Opened, RepositoryVisibility.Internal)] + [InlineData(Updated, RepositoryVisibility.Internal)] + public async Task On_a_public_or_internal_repository_an_existing_activation_admits_only_members_and_records_the_refusal_once(string typeKey, RepositoryVisibility visibility) + { + // Internal too: every signed-in user of the instance (GitLab) or member of the enterprise (GitHub) can read an + // internal repository, fork it and open a pull request from the fork. + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, typeKey, "{}"); + var outsider = new PullRequestOrigin { AuthorAssociation = PullRequestAuthorAssociation.None, IsFork = true, HeadRepositoryFullName = "evil/repo", RepositoryVisibility = visibility }; + + await PublishAsync(Event(typeKey, seed.RepositoryId, number: 1, outsider)); + await PublishAsync(Event(typeKey, seed.RepositoryId, number: 1, outsider)); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(0); + (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.AuthorNotMember)).Count.ShouldBe(1, customMessage: "a reopen loop is a stream of genuinely signed deliveries: one row per (activation, PR) per day"); + (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.NoMatchingActivation)).ShouldBeEmpty("a refusal is its own reason — it must not also read as nothing listening"); + } + + [Theory] + [InlineData(PullRequestAuthorAssociation.Member, "{}", 1)] + [InlineData(PullRequestAuthorAssociation.Contributor, "{}", 0)] + [InlineData(PullRequestAuthorAssociation.Unknown, "{}", 0)] + [InlineData(PullRequestAuthorAssociation.None, """{"authors":"any"}""", 1)] + public async Task On_a_public_repository_the_authors_filter_decides(PullRequestAuthorAssociation association, string configJson, int expectedRuns) + { + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, Opened, configJson); + + await PublishAsync(Event(Opened, seed.RepositoryId, number: 2, new PullRequestOrigin { AuthorAssociation = association, RepositoryVisibility = RepositoryVisibility.Public })); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(expectedRuns); + } + + [Fact] + public async Task On_a_private_repository_an_existing_activation_keeps_admitting_anyone_and_members_can_still_be_chosen() + { + var seed = await SeedAsync(ProviderKind.GitHub); + var membersOnly = await SeedSiblingWorkflowAsync(seed); + await SeedActivationAsync(seed, Opened, "{}"); + await SeedActivationAsync(membersOnly, Opened, """{"authors":"members"}"""); + + await PublishAsync(Event(Opened, seed.RepositoryId, number: 3, new PullRequestOrigin { AuthorAssociation = PullRequestAuthorAssociation.None, IsFork = true, RepositoryVisibility = RepositoryVisibility.Private })); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(1, customMessage: "the default only narrows on PUBLIC repositories"); + (await CountRunsAsync(membersOnly.WorkflowId)).ShouldBe(0, customMessage: "an explicit members choice holds on any repository"); + } + + [Theory] + [InlineData("66", true, 0)] // an outsider pushed to a member's PR whose head lives in the outsider's fork + [InlineData("9", true, 1)] // the member author pushed to their own fork PR + [InlineData("66", false, 1)] // someone else pushed to a head in the target repository, which takes a role there + public async Task On_a_public_repository_new_commits_are_admitted_only_when_the_author_or_a_member_pushed_them(string pusher, bool isFork, int expectedRuns) + { + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, Updated, "{}"); + var origin = new PullRequestOrigin { AuthorExternalId = "9", AuthorAssociation = PullRequestAuthorAssociation.Member, IsFork = isFork, HeadRepositoryFullName = isFork ? "evil/repo" : "acme/api", RepositoryVisibility = RepositoryVisibility.Public, PusherExternalId = pusher }; + + await PublishAsync(Event(Updated, seed.RepositoryId, number: 13, origin)); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(expectedRuns, customMessage: "a member's PR from an outsider's fork carries the outsider's code once the outsider pushes — check PullRequestTriggerAuthors.AdmitsPusher"); + if (expectedRuns == 0) (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.AuthorNotMember)).ShouldHaveSingleItem().ShouldContain("pushed by"); + } + + [Fact] + public async Task The_run_payload_says_who_wrote_the_pr_and_where_its_head_lives() + { + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, Updated, """{"authors":"any"}"""); + + await PublishAsync(Event(Updated, seed.RepositoryId, number: 4, new PullRequestOrigin { AuthorAssociation = PullRequestAuthorAssociation.Contributor, IsFork = true, HeadRepositoryFullName = "evil/repo", RepositoryVisibility = RepositoryVisibility.Public })); + + var payload = await LoadOnlyPayloadAsync(seed.WorkflowId); + payload.GetProperty("authorAssociation").GetString().ShouldBe("contributor"); + payload.GetProperty("isFork").GetBoolean().ShouldBeTrue(); + payload.GetProperty("headRepositoryFullName").GetString().ShouldBe("evil/repo"); + } + + // ─── GitLab standing ─────────────────────────────────────────────────────── + + [Theory] + [InlineData(200, """{"id":51,"access_level":40}""", 1, "member")] + [InlineData(200, """{"id":51,"access_level":20}""", 0, null)] + [InlineData(404, """{"message":"404 Not found"}""", 0, null)] + [InlineData(500, """{"message":"boom"}""", 0, null)] + public async Task On_a_public_gitlab_project_the_authors_standing_is_asked_of_gitlab(int status, string memberBody, int expectedRuns, string? expectedAssociation) + { + using var gitlab = new StubProviderHost(); + var seed = await SeedAsync(ProviderKind.GitLab, gitlab.BaseUrl); + gitlab.Answer("GET", $"/api/v4/projects/{seed.ExternalId}/members/all/51", status, memberBody); + await SeedActivationAsync(seed, Opened, "{}"); + + await PublishAsync(Event(Opened, seed.RepositoryId, number: 5, new PullRequestOrigin { AuthorExternalId = "51", RepositoryVisibility = RepositoryVisibility.Public, IsFork = true })); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(expectedRuns, customMessage: "Developer and above is a member; Reporter, not a member at all, and an answer GitLab could not give are not"); + if (expectedAssociation != null) (await LoadOnlyPayloadAsync(seed.WorkflowId)).GetProperty("authorAssociation").GetString().ShouldBe(expectedAssociation); + gitlab.Requests.ShouldContain(r => r.PrivateTokenHeader == "glpat-fake-test-token", customMessage: "the lookup authenticates as the repository's own credential"); + } + + [Theory] + [InlineData(200, """{"id":66,"access_level":30}""", 1)] + [InlineData(404, """{"message":"404 Not found"}""", 0)] + public async Task On_a_public_gitlab_project_a_push_by_someone_else_to_a_fork_mr_asks_gitlab_about_the_pusher(int status, string pusherBody, int expectedRuns) + { + using var gitlab = new StubProviderHost(); + var seed = await SeedAsync(ProviderKind.GitLab, gitlab.BaseUrl); + gitlab.Answer("GET", $"/api/v4/projects/{seed.ExternalId}/members/all/51", 200, """{"id":51,"access_level":40}""") + .Answer("GET", $"/api/v4/projects/{seed.ExternalId}/members/all/66", status, pusherBody); + await SeedActivationAsync(seed, Updated, "{}"); + + await PublishAsync(Event(Updated, seed.RepositoryId, number: 14, new PullRequestOrigin { AuthorExternalId = "51", RepositoryVisibility = RepositoryVisibility.Public, IsFork = true, PusherExternalId = "66" })); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(expectedRuns, customMessage: "a Developer pushing to a member's fork MR is a member's push; someone with no role on the project is not"); + gitlab.Requests.Count(r => r.PathAndQuery.Contains("/members/all/66")).ShouldBe(1, customMessage: "GitLab names the pusher but not their standing, so it is asked like the author's"); + } + + [Fact] + public async Task One_delivery_asks_gitlab_once_however_many_activations_match_and_an_any_trigger_still_learns_the_standing() + { + using var gitlab = new StubProviderHost(); + var seed = await SeedAsync(ProviderKind.GitLab, gitlab.BaseUrl); + var sibling = await SeedSiblingWorkflowAsync(seed); + gitlab.Answer("GET", $"/api/v4/projects/{seed.ExternalId}/members/all/51", 200, """{"id":51,"access_level":30}"""); + await SeedActivationAsync(seed, Opened, """{"authors":"any"}"""); + await SeedActivationAsync(sibling, Opened, """{"authors":"members"}"""); + + await PublishAsync(Event(Opened, seed.RepositoryId, number: 6, new PullRequestOrigin { AuthorExternalId = "51", RepositoryVisibility = RepositoryVisibility.Private })); + + gitlab.Requests.Count(r => r.PathAndQuery.Contains("/members/all/51")).ShouldBe(1); + (await CountRunsAsync(sibling.WorkflowId)).ShouldBe(1); + (await LoadOnlyPayloadAsync(seed.WorkflowId)).GetProperty("authorAssociation").GetString().ShouldBe("member", "the payload says who wrote the PR whichever filter the activation chose"); + } + + [Fact] + public async Task A_lookup_gitlab_could_not_answer_is_asked_once_per_delivery_not_once_per_activation() + { + using var gitlab = new StubProviderHost(); + var seed = await SeedAsync(ProviderKind.GitLab, gitlab.BaseUrl); + var sibling = await SeedSiblingWorkflowAsync(seed); + gitlab.Answer("GET", $"/api/v4/projects/{seed.ExternalId}/members/all/51", 500, """{"message":"boom"}"""); + await SeedActivationAsync(seed, Opened, """{"authors":"members"}"""); + await SeedActivationAsync(sibling, Opened, """{"authors":"members"}"""); + + await PublishAsync(Event(Opened, seed.RepositoryId, number: 11, new PullRequestOrigin { AuthorExternalId = "51", RepositoryVisibility = RepositoryVisibility.Public })); + + gitlab.Requests.Count(r => r.PathAndQuery.Contains("/members/all/51")).ShouldBe(1, customMessage: "an unanswered lookup costs up to its timeout; every matching activation repeating it would hold the delivery past the provider's own timeout"); + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(0); + (await CountRunsAsync(sibling.WorkflowId)).ShouldBe(0); + } + + // ─── Debounce ────────────────────────────────────────────────────────────── + + [Theory] + [InlineData(Opened)] + [InlineData(Updated)] + public async Task A_burst_for_one_head_starts_one_run_per_activation_and_records_the_debounce_once(string typeKey) + { + // A close/reopen loop: every reopen is a genuinely signed delivery for a head that already ran. + var seed = await SeedAsync(ProviderKind.GitHub); + var sibling = await SeedSiblingWorkflowAsync(seed); + await SeedActivationAsync(seed, typeKey, "{}"); + await SeedActivationAsync(sibling, typeKey, "{}"); + + for (var i = 0; i < 4; i++) + await PublishAsync(Event(typeKey, seed.RepositoryId, number: 7, new PullRequestOrigin())); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(1); + (await CountRunsAsync(sibling.WorkflowId)).ShouldBe(1, customMessage: "the debounce is per activation: one workflow's run must not swallow another's"); + (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.PullRequestDebounced)).Count.ShouldBe(2, customMessage: "one row per (activation, PR, head) per day, however many events the burst held"); + } + + [Theory] + [InlineData(Opened)] + [InlineData(Updated)] + public async Task A_new_head_inside_the_window_still_starts_its_own_run(string typeKey) + { + // A member amending a push seconds later: the commit that ends up on the PR must get a run, on any repository. + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, typeKey, "{}"); + + await PublishAsync(Event(typeKey, seed.RepositoryId, number: 15, new PullRequestOrigin { AuthorAssociation = PullRequestAuthorAssociation.Member, RepositoryVisibility = RepositoryVisibility.Private }, headSha: "sha-first")); + await PublishAsync(Event(typeKey, seed.RepositoryId, number: 15, new PullRequestOrigin { AuthorAssociation = PullRequestAuthorAssociation.Member, RepositoryVisibility = RepositoryVisibility.Private }, headSha: "sha-amended")); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(2, customMessage: "the debounce holds off the same head, never newer code — check PullRequestTriggerAdmission.BuildDebounceKey"); + (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.PullRequestDebounced)).ShouldBeEmpty(); + if (typeKey == Updated) (await LoadPayloadsAsync(seed.WorkflowId, "newHeadSha")).ShouldBe(new[] { "sha-first", "sha-amended" }, ignoreOrder: true); + } + + [Fact] + public async Task A_delivery_posted_again_after_the_window_holds_off_nothing() + { + // The delivery already started its run, so posting it again — an operator's Redeliver, or a captured body with + // its own id — must not take the debounce claim a second time and swallow the next real event for that head. + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, Opened, "{}"); + var original = Event(Opened, seed.RepositoryId, number: 16, new PullRequestOrigin()); + + await PublishAsync(original); + await ExpireDebounceClaimsAsync(seed.RepositoryId); + await PublishAsync(Event(Opened, seed.RepositoryId, number: 16, new PullRequestOrigin(), original.ProviderEventId)); + await PublishAsync(Event(Opened, seed.RepositoryId, number: 16, new PullRequestOrigin())); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(2, customMessage: "the reposted delivery is a duplicate and must leave the window alone — check the already-started check in RunSourceDispatcher.FireIfMatchesAsync"); + (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.PullRequestDebounced)).ShouldBeEmpty(); + } + + [Fact] + public async Task Two_deliveries_for_one_pr_racing_each_other_start_one_run_per_activation() + { + var seed = await SeedAsync(ProviderKind.GitHub); + var sibling = await SeedSiblingWorkflowAsync(seed); + await SeedActivationAsync(seed, Opened, "{}"); + await SeedActivationAsync(sibling, Opened, "{}"); + + using var firstScope = _fixture.BeginScope(); + using var secondScope = _fixture.BeginScope(); + await using var first = await firstScope.Resolve().Database.BeginTransactionAsync(); + await using var second = await secondScope.Resolve().Database.BeginTransactionAsync(); + + await firstScope.Resolve().Publish(Event(Opened, seed.RepositoryId, number: 12, new PullRequestOrigin())); + var contender = secondScope.Resolve().Publish(Event(Opened, seed.RepositoryId, number: 12, new PullRequestOrigin())); + + await Task.Delay(200); + contender.IsCompleted.ShouldBeFalse("the second delivery must wait on the first's uncommitted debounce claim, not read past it"); + + await first.CommitAsync(); + await contender.WaitAsync(TimeSpan.FromSeconds(10)); + await second.CommitAsync(); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(1); + (await CountRunsAsync(sibling.WorkflowId)).ShouldBe(1, customMessage: "a reopen racing a reopen is still one run per activation — check the ON CONFLICT ... WHERE in WebhookClaimStore and the activation order in RunSourceDispatcher.LoadActiveActivationsAsync"); + (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.PullRequestDebounced)).Count.ShouldBe(2); + } + + [Fact] + public async Task A_redelivery_of_the_same_delivery_is_a_duplicate_not_a_debounce() + { + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, Opened, "{}"); + var delivery = Event(Opened, seed.RepositoryId, number: 8, new PullRequestOrigin()); + + await PublishAsync(delivery); + await PublishAsync(Event(Opened, seed.RepositoryId, number: 8, new PullRequestOrigin(), delivery.ProviderEventId)); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(1); + (await LoadRefusalsAsync(seed.TeamId, WorkflowRunRequestRejectionReasons.PullRequestDebounced)).ShouldBeEmpty("the same delivery id holds the debounce claim, so the provider redelivering it is deduplicated by the run's idempotency key, as it always was"); + } + + [Fact] + public async Task A_different_pull_request_is_not_debounced() + { + var seed = await SeedAsync(ProviderKind.GitHub); + await SeedActivationAsync(seed, Opened, "{}"); + + await PublishAsync(Event(Opened, seed.RepositoryId, number: 9, new PullRequestOrigin())); + await PublishAsync(Event(Opened, seed.RepositoryId, number: 10, new PullRequestOrigin())); + + (await CountRunsAsync(seed.WorkflowId)).ShouldBe(2); + } + + // ─── Events ──────────────────────────────────────────────────────────────── + + /// One head unless a test says otherwise, so a repeated event is the same PR state arriving again — a reopen, a redelivery. + private static NormalizedEvent Event(string typeKey, Guid repositoryId, int number, PullRequestOrigin origin, string? deliveryId = null, string headSha = "head-1") + { + var id = deliveryId ?? $"d-{Guid.NewGuid():N}"; + + if (typeKey == Updated) + return new PullRequestSynchronizedEvent { RepositoryId = repositoryId, ProviderEventId = id, OccurredAt = DateTimeOffset.UtcNow, ExternalPullRequestId = $"{number}", Number = number, PreviousHeadSha = "a", NewHeadSha = headSha, Origin = origin }; + + return new PullRequestOpenedEvent { RepositoryId = repositoryId, ProviderEventId = id, OccurredAt = DateTimeOffset.UtcNow, ExternalPullRequestId = $"{number}", Number = number, Title = "t", SourceBranch = "main", TargetBranch = "main", AuthorExternalId = "9", AuthorName = "someone", WebUrl = "https://x", HeadSha = headSha, Origin = origin }; + } + + /// The window passing, without waiting it out: every debounce claim on this repository ends a minute ago. + private async Task ExpireDebounceClaimsAsync(Guid repositoryId) + { + using var scope = _fixture.BeginScope(); + var pattern = $"pr-debounce:%:{repositoryId:N}:%"; + await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE webhook_claim SET claimed_at = now() - interval '2 minutes', expires_at = now() - interval '1 minute' WHERE claim_key LIKE {pattern}"); + } + + private async Task PublishAsync(NormalizedEvent ev) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().Publish(ev); + } + + // ─── Seeds ───────────────────────────────────────────────────────────────── + + private sealed record Seed(Guid TeamId, Guid UserId, Guid WorkflowId, Guid RepositoryId, string ExternalId); + + private async Task SeedAsync(ProviderKind provider, string? baseUrl = null) + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var workflowId = await CreateWorkflowAsync(teamId, userId); + var externalId = Random.Shared.Next(100_000, 999_999).ToString(); + var repositoryId = Guid.NewGuid(); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = teamId, Provider = provider, DisplayName = "P", BaseUrl = baseUrl ?? $"https://p-{Guid.NewGuid():N}.invalid", ApiUrl = baseUrl }; + var credential = new Credential { Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instance.Id, AuthType = AuthType.Pat, DisplayName = "PAT", EncryptedPayload = scope.Resolve().Encrypt("{\"token\":\"glpat-fake-test-token\"}") }; + + db.ProviderInstance.Add(instance); + db.Credential.Add(credential); + db.Repository.Add(new Repository { Id = repositoryId, TeamId = teamId, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ExternalId = externalId, NamespacePath = "acme", Name = "api", FullPath = "acme/api", WebUrl = "https://x/acme/api" }); + await db.SaveChangesAsync(); + + return new Seed(teamId, userId, workflowId, repositoryId, externalId); + } + + /// A second member of the same team publishing a second workflow — a delivery only ever reaches its own team. + private async Task SeedSiblingWorkflowAsync(Seed seed) + { + var userId = Guid.NewGuid(); + + using (var scope = _fixture.BeginScope()) + { + var db = scope.Resolve(); + db.User.Add(new User { Id = userId, Email = $"sibling-{userId:N}@test.local", Name = $"sibling-{userId:N}", CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = seed.TeamId, UserId = userId, Role = TeamRole.Admin, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + await db.SaveChangesAsync(); + } + + return seed with { UserId = userId, WorkflowId = await CreateWorkflowAsync(seed.TeamId, userId) }; + } + + private async Task CreateWorkflowAsync(Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); + return await scope.Resolve().Send(new CreateWorkflowCommand { Name = "admission-" + Guid.NewGuid().ToString("N")[..8], Description = null, Definition = WorkflowsTestSeed.MinimalDefinition(), Activations = new List(), Enabled = true }); + } + + private async Task SeedActivationAsync(Seed seed, string typeKey, string configJson) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + db.WorkflowActivation.Add(new WorkflowActivation { Id = Guid.NewGuid(), WorkflowId = seed.WorkflowId, TypeKey = typeKey, ConfigJson = configJson, Enabled = true, CreatedBy = seed.UserId, LastModifiedBy = seed.UserId }); + await db.SaveChangesAsync(); + } + + // ─── Reads ───────────────────────────────────────────────────────────────── + + private async Task CountRunsAsync(Guid workflowId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRun.AsNoTracking().CountAsync(r => r.WorkflowId == workflowId); + } + + private async Task LoadOnlyPayloadAsync(Guid workflowId) + { + using var scope = _fixture.BeginScope(); + var payload = await scope.Resolve().WorkflowRun.AsNoTracking().Where(r => r.WorkflowId == workflowId).Select(r => r.RunRequest.NormalizedPayloadJson).SingleAsync(); + return JsonDocument.Parse(payload).RootElement; + } + + private async Task> LoadPayloadsAsync(Guid workflowId, string field) + { + using var scope = _fixture.BeginScope(); + var payloads = await scope.Resolve().WorkflowRun.AsNoTracking().Where(r => r.WorkflowId == workflowId).Select(r => r.RunRequest.NormalizedPayloadJson).ToListAsync(); + return payloads.Select(p => JsonDocument.Parse(p).RootElement.GetProperty(field).GetString()!).ToList(); + } + + private async Task> LoadRefusalsAsync(Guid teamId, string reason) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunRequest.AsNoTracking() + .Where(r => r.TeamId == teamId && r.Status == WorkflowRunRequestStatus.Rejected && r.Error!.StartsWith(reason + ":")) + .Select(r => r.Error!) + .ToListAsync(); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs index 80a9058f6..576dc11b9 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs @@ -128,6 +128,7 @@ public async Task Budget_settlement_releases_the_terminal_map_branch_reservation [InlineData(typeof(ProbeStaleModelAvailabilityCommand))] [InlineData(typeof(ProbeStaleStorageDestinationsCommand))] [InlineData(typeof(ProbeUnknownModelCapabilitiesCommand))] + [InlineData(typeof(PurgeExpiredWebhookClaimsCommand))] [InlineData(typeof(ReapAgentRunOrphansCommand))] [InlineData(typeof(ReapAgentRunSpoolsCommand))] [InlineData(typeof(ReapUnreferencedArtifactsCommand))] diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs index 208578f7f..cf67f4d3b 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs @@ -36,6 +36,7 @@ public class RequestAuthorizationInventoryTests ["ReceiveWebhookCommand"] = "provider-signed inbound; proof is the HMAC verified against the per-webhook secret.", ["ReceiveConnectionWebhookCommand"] = "the same inbound, from a group/organization hook; proof is the HMAC verified against THAT hook's own secret, never a repository's.", ["ResumeWorkflowCallbackCommand"] = "proof is the single-use high-entropy callback token in the URL.", + ["WebhookExistsQuery"] = "asked before a delivery's body is read; it answers only whether the hook id names a hook, which the delivery's own 404 already tells the caller.", ["CompleteCredentialOAuthCommand"] = "the provider redirect carries no JWT; proof is the one-time OAuthPendingState row.", ["AcceptInvitationCommand"] = "the invitee has no account yet; proof is the single-use invitation token in the route.", ["PreviewInvitationQuery"] = "same token, read-only, and it answers nothing at all unless the token checks out.", @@ -58,6 +59,7 @@ public class RequestAuthorizationInventoryTests ["ProbeStaleStorageDestinationsCommand"] = "re-probes the destinations Active routes bind writes to and the destinations that still hold stored objects, across every team; a per-team capability would make the deployment-wide question unaskable, and it changes no row a user owns — only the observed health of a destination.", ["VerifyStaleArtifactLocationsCommand"] = "re-reads the least recently verified artifact placements across every team, because whether bytes still sit at a destination is a question no team can ask on its own behalf and no reader discovers until it is too late; it never touches a row a user authored, only the observed state of a placement, and it can restore as well as demote.", ["ProbeUnknownModelCapabilitiesCommand"] = "sweep", + ["PurgeExpiredWebhookClaimsCommand"] = "deletes webhook-ingress claims whose window has ended, across every hook, in bounded batches; a claim belongs to a delivery, not to a user, and an expired one holds nothing off — the next claim on its key would take it over anyway.", ["ProjectWorkflowRunModelCallsCommand"] = "projects started and terminal workflow-run interaction evidence; it is dispatched only by the bounded system recurring job.", ["ProjectWorkflowRunToolCallsCommand"] = "projects governed side-effect ledger facts into the observation-only Workflow Run tool-call plane; it is dispatched only by the bounded system recurring job.", ["MaterializeWorkflowRunModelCallBodiesCommand"] = "materializes already-declared telemetry bodies in bounded lease/fence batches; it changes no Workflow Run outcome.", diff --git a/backend/tests/CodeSpace.UnitTests/Jobs/PeriodicJanitorDispatchTests.cs b/backend/tests/CodeSpace.UnitTests/Jobs/PeriodicJanitorDispatchTests.cs index 1aee30562..643a502cd 100644 --- a/backend/tests/CodeSpace.UnitTests/Jobs/PeriodicJanitorDispatchTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Jobs/PeriodicJanitorDispatchTests.cs @@ -1,20 +1,23 @@ using CodeSpace.Core.Handlers.CommandHandlers.Auth; using CodeSpace.Core.Handlers.CommandHandlers.Credentials; +using CodeSpace.Core.Handlers.CommandHandlers.Webhooks; using CodeSpace.Core.Jobs.RecurringJobs; using CodeSpace.Core.Services.Auth; using CodeSpace.Core.Services.OAuth; +using CodeSpace.Core.Services.Webhooks; using CodeSpace.Messages.Commands.Auth; using CodeSpace.Messages.Commands.OAuth; +using CodeSpace.Messages.Commands.Webhooks; using MediatR; using Shouldly; namespace CodeSpace.UnitTests.Jobs; /// -/// 🟢 Unit: the two periodic janitors that used to be BackgroundServices are now the standard -/// job → command → service chain (Rule 14 + Rule 16). Each job sends exactly its command on its cadence and holds no -/// logic; each handler forwards to its service and returns its count and holds no logic. Hand-rolled recording -/// doubles (no mocking lib, matching the codebase convention). +/// 🟢 Unit: the periodic janitors — two that used to be BackgroundServices, and the webhook-claim sweep — are the +/// standard job → command → service chain (Rule 14 + Rule 16). Each job sends exactly its command on its cadence and +/// holds no logic; each handler forwards to its service and returns its count and holds no logic. Hand-rolled +/// recording doubles (no mocking lib, matching the codebase convention). /// [Trait("Category", "Unit")] public class PeriodicJanitorDispatchTests @@ -71,6 +74,32 @@ public async Task The_unrotated_password_handler_forwards_to_the_audit_and_retur result.Unrotated.ShouldBe(2, "the handler surfaces the audit's unrotated count verbatim"); } + [Fact] + public async Task The_webhook_claim_sweep_dispatches_its_command_every_five_minutes() + { + var mediator = new RecordingMediator(); + var job = new WebhookClaimPurgeRecurringJob(mediator); + + job.JobId.ShouldBe(nameof(WebhookClaimPurgeRecurringJob)); + job.CronExpression.ShouldBe("*/5 * * * *", "claims are taken over in place when they lapse, so the sweep only bounds the table; five minutes keeps it near one window of deliveries"); + + await job.Execute(); + + mediator.Sent.ShouldHaveSingleItem().ShouldBeOfType("the job is a thin dispatcher — it only sends the command"); + } + + [Fact] + public async Task The_webhook_claim_handler_forwards_to_the_claim_store_and_returns_its_count() + { + var claims = new RecordingClaimStore { ToReturn = 3 }; + var handler = new PurgeExpiredWebhookClaimsCommandHandler(claims); + + var result = await handler.Handle(new PurgeExpiredWebhookClaimsCommand(), CancellationToken.None); + + claims.PurgeCalls.ShouldBe(1, "the handler delegates the whole sweep to the store (Rule 16)"); + result.Deleted.ShouldBe(3, "the handler surfaces the store's deleted count verbatim"); + } + /// Records the requests sent through the mediator; the rest of the surface is unreachable in these tests. private sealed class RecordingMediator : IMediator { @@ -113,6 +142,21 @@ public Task DeleteExpiredAsync(CancellationToken cancellationToken) } } + /// Records the sweep call + returns a canned count; a claim is never taken in these tests. + private sealed class RecordingClaimStore : IWebhookClaimStore + { + public int PurgeCalls; + public int ToReturn; + + public Task TryClaimAsync(string key, string holder, TimeSpan holdFor, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task PurgeExpiredAsync(CancellationToken cancellationToken) + { + PurgeCalls++; + return Task.FromResult(ToReturn); + } + } + /// Records the audit call + returns a canned count, so the handler test asserts pure delegation. private sealed class RecordingPasswordAudit : IUnrotatedBootstrapPasswordAudit { diff --git a/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubPullRequestOriginTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubPullRequestOriginTests.cs new file mode 100644 index 000000000..3c575bf0f --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubPullRequestOriginTests.cs @@ -0,0 +1,129 @@ +using CodeSpace.Core.Services.Providers.Events; +using CodeSpace.Core.Services.Providers.GitHub; +using CodeSpace.Core.Services.Providers.GitHub.Events; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Events.PullRequest; +using Shouldly; + +namespace CodeSpace.UnitTests.Providers.GitHub; + +/// +/// What a GitHub pull_request delivery says about who wrote the PR and where its head lives. Before this, a fork PR from +/// an outsider whose head branch was named main normalised to exactly what a member's PR from the base repository +/// did — the trigger payload carried no association, no fork bit and no head repository, so no workflow could gate on it. +/// +[Trait("Category", "Unit")] +public class GitHubPullRequestOriginTests +{ + private readonly GitHubEventNormalizer _normalizer = new(new ProviderEventSubscriptionRegistry(new IProviderEventSubscription[] { new GitHubPullRequestEventSubscription() })); + private readonly Guid _repositoryId = Guid.NewGuid(); + + [Theory] + [InlineData("opened")] + [InlineData("reopened")] + public void An_outsider_fork_pr_carries_its_origin(string action) + { + var opened = Normalize(Body(action, association: "NONE", headRepo: """{"id":222,"full_name":"evil/repo","fork":true}""", visibility: "public")); + + opened.Origin.ShouldBe(new PullRequestOrigin { AuthorExternalId = "9", AuthorAssociation = PullRequestAuthorAssociation.None, IsFork = true, HeadRepositoryFullName = "evil/repo", RepositoryVisibility = RepositoryVisibility.Public }); + } + + [Fact] + public void A_synchronize_carries_the_same_origin_as_an_open_and_names_who_pushed() + { + // sender is whoever pushed the commits — on a member's PR from an outsider's fork, the outsider. + var synced = Normalize(Body("synchronize", association: "COLLABORATOR", headRepo: """{"id":111,"full_name":"acme/api","fork":false}""", visibility: "private")); + + synced.Origin.ShouldBe(new PullRequestOrigin { AuthorExternalId = "9", AuthorAssociation = PullRequestAuthorAssociation.Member, IsFork = false, HeadRepositoryFullName = "acme/api", RepositoryVisibility = RepositoryVisibility.Private, PusherExternalId = "66" }); + ((IPullRequestOriginEvent)synced).HeadSha.ShouldBe("b", "the head a synchronize is about is the commit it moved the PR to"); + } + + [Theory] + [InlineData("opened")] + [InlineData("reopened")] + public void An_open_names_its_head_commit_and_no_pusher(string action) + { + // sender on a reopen is whoever reopened it, not someone who pushed code — it must not stand in for a pusher. + var opened = Normalize(Body(action, association: "MEMBER", headRepo: """{"id":111,"full_name":"acme/api","fork":false}""", visibility: "public")); + + opened.HeadSha.ShouldBe("s"); + opened.Origin.PusherExternalId.ShouldBeNull(); + } + + [Fact] + public void A_base_repository_that_is_itself_a_fork_is_not_a_fork_pr() + { + // head.repo.fork says the head REPOSITORY is a fork of something — true for every branch of an org's own fork of + // upstream. Whether the PR crosses repositories is whether head and base are the same repository. + var opened = Normalize(Body("opened", association: "MEMBER", headRepo: """{"id":111,"full_name":"acme/api","fork":true}""", visibility: "public")); + + opened.Origin.IsFork.ShouldBeFalse(); + } + + [Fact] + public void A_deleted_fork_head_is_a_fork_with_no_head_repository() + { + var opened = Normalize(Body("opened", association: "NONE", headRepo: "null", visibility: "public")); + + opened.Origin.IsFork.ShouldBeTrue("GitHub sends head.repo: null only when the fork the PR came from was deleted"); + opened.Origin.HeadRepositoryFullName.ShouldBeNull(); + } + + [Fact] + public void A_payload_without_origin_fields_knows_nothing_and_is_not_called_a_fork() + { + const string body = """{"action":"opened","pull_request":{"id":1,"number":7,"title":"t","head":{"ref":"f","sha":"s"},"base":{"ref":"main"},"user":{"id":5,"login":"u"},"html_url":"x"} }"""; + + var opened = Normalize(body); + + opened.Origin.ShouldBe(new PullRequestOrigin { AuthorExternalId = "5", AuthorAssociation = PullRequestAuthorAssociation.Unknown, IsFork = false, HeadRepositoryFullName = null, RepositoryVisibility = null }); + } + + [Theory] + [InlineData("OWNER", PullRequestAuthorAssociation.Member)] + [InlineData("MEMBER", PullRequestAuthorAssociation.Member)] + [InlineData("COLLABORATOR", PullRequestAuthorAssociation.Member)] + [InlineData("CONTRIBUTOR", PullRequestAuthorAssociation.Contributor)] + [InlineData("FIRST_TIME_CONTRIBUTOR", PullRequestAuthorAssociation.Contributor)] + [InlineData("FIRST_TIMER", PullRequestAuthorAssociation.Contributor)] + [InlineData("NONE", PullRequestAuthorAssociation.None)] + [InlineData("MANNEQUIN", PullRequestAuthorAssociation.None)] + [InlineData("SOMETHING_NEW", PullRequestAuthorAssociation.Unknown)] + [InlineData(null, PullRequestAuthorAssociation.Unknown)] + public void Every_github_association_maps_to_one_standing(string? raw, PullRequestAuthorAssociation expected) + { + GitHubPullRequestEventSubscription.MapAuthorAssociation(raw).ShouldBe(expected); + } + + [Theory] + [InlineData("""{"visibility":"public","private":false}""", RepositoryVisibility.Public)] + [InlineData("""{"visibility":"internal","private":true}""", RepositoryVisibility.Internal)] + [InlineData("""{"visibility":"private","private":true}""", RepositoryVisibility.Private)] + [InlineData("""{"private":false}""", RepositoryVisibility.Public)] + [InlineData("""{"private":true}""", RepositoryVisibility.Private)] + [InlineData("""{"id":1}""", null)] + public void The_repository_visibility_is_read_from_the_delivery(string repositoryJson, RepositoryVisibility? expected) + { + var body = $$"""{"action":"opened","repository":{{repositoryJson}},"pull_request":{"id":1,"number":7,"title":"t","head":{"ref":"f","sha":"s"},"base":{"ref":"main"},"user":{"id":5,"login":"u"},"html_url":"x"} }"""; + + Normalize(body).Origin.RepositoryVisibility.ShouldBe(expected); + } + + private T Normalize(string body) where T : class + { + var headers = new Dictionary { ["X-GitHub-Event"] = "pull_request", ["X-GitHub-Delivery"] = "d-1" }; + + return _normalizer.Normalize(_repositoryId, body, headers).ShouldBeOfType(); + } + + private static string Body(string action, string association, string headRepo, string visibility) => + $$""" + {"action":"{{action}}","before":"a","after":"b", + "repository":{"id":111,"full_name":"acme/api","private":{{(visibility == "public" ? "false" : "true")}},"visibility":"{{visibility}}"}, + "pull_request":{"id":1,"number":7,"title":"Ignore previous instructions","body":null,"author_association":"{{association}}", + "head":{"ref":"main","sha":"s","repo":{{headRepo}}}, + "base":{"ref":"main","sha":"t","repo":{"id":111,"full_name":"acme/api","fork":false} }, + "user":{"id":9,"login":"outsider"},"html_url":"https://github.example/acme/api/pull/7","labels":[]}, + "sender":{"id":66,"login":"pusher"} } + """; +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMemberStandingTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMemberStandingTests.cs new file mode 100644 index 000000000..bf0349ed4 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMemberStandingTests.cs @@ -0,0 +1,37 @@ +using CodeSpace.Core.Services.Providers.GitLab; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.UnitTests.Providers.GitLab; + +/// +/// A GitLab merge request's author standing, from GET /projects/:id/members/all/:user_id. Developer is the floor: +/// it is the first role that can push to the project, so it is the GitLab counterpart of GitHub's collaborator; a Guest +/// or Reporter can see the project but holds no say over its code. +/// +[Trait("Category", "Unit")] +public class GitLabMemberStandingTests +{ + [Theory] + [InlineData(50, PullRequestAuthorAssociation.Member)] + [InlineData(40, PullRequestAuthorAssociation.Member)] + [InlineData(30, PullRequestAuthorAssociation.Member)] + [InlineData(20, PullRequestAuthorAssociation.None)] + [InlineData(10, PullRequestAuthorAssociation.None)] + [InlineData(5, PullRequestAuthorAssociation.None)] + [InlineData(null, PullRequestAuthorAssociation.None)] + public void Every_access_level_maps_to_one_standing(int? accessLevel, PullRequestAuthorAssociation expected) + { + GitLabRepositoryProvider.MapMemberStanding(accessLevel).ShouldBe(expected); + } + + [Theory] + [InlineData("""{"id":51,"username":"dev","access_level":30,"state":"active"}""", 30)] + [InlineData("""{"id":51,"username":"dev"}""", null)] + [InlineData("""{"access_level":"30"}""", null)] + [InlineData("not json", null)] + public void The_access_level_is_read_from_the_member_body(string json, int? expected) + { + GitLabRepositoryProvider.ParseMemberAccessLevel(json).ShouldBe(expected); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMergeRequestOriginTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMergeRequestOriginTests.cs new file mode 100644 index 000000000..17bb8b61f --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabMergeRequestOriginTests.cs @@ -0,0 +1,100 @@ +using CodeSpace.Core.Services.Providers.Events; +using CodeSpace.Core.Services.Providers.GitLab; +using CodeSpace.Core.Services.Providers.GitLab.Events; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Events.PullRequest; +using Shouldly; + +namespace CodeSpace.UnitTests.Providers.GitLab; + +/// +/// What a GitLab Merge Request Hook says about who wrote the MR and where its source branch lives. GitLab's payload names +/// the author but not their standing, so the association stays Unknown here and is looked up at dispatch; the fork bit, +/// the source project and the project's visibility are all on the payload. +/// +[Trait("Category", "Unit")] +public class GitLabMergeRequestOriginTests +{ + private readonly GitLabEventNormalizer _normalizer = new(new ProviderEventSubscriptionRegistry(new IProviderEventSubscription[] { new GitLabMergeRequestEventSubscription() })); + private readonly Guid _repositoryId = Guid.NewGuid(); + + [Theory] + [InlineData("open")] + [InlineData("reopen")] + public void A_merge_request_from_a_fork_carries_its_origin(string action) + { + var opened = Normalize(Body(action, sourceProjectId: 16, visibilityLevel: 20)); + + opened.Origin.ShouldBe(new PullRequestOrigin { AuthorExternalId = "51", AuthorAssociation = PullRequestAuthorAssociation.Unknown, IsFork = true, HeadRepositoryFullName = "evil/api", RepositoryVisibility = RepositoryVisibility.Public }); + } + + [Fact] + public void The_author_is_the_merge_requests_author_not_whoever_reopened_it() + { + // user is the actor of THIS delivery; a maintainer reopening an outsider's MR must not lend the MR their standing. + var opened = Normalize(Body("reopen", sourceProjectId: 15, visibilityLevel: 0)); + + opened.AuthorName.ShouldBe("maintainer"); + opened.Origin.AuthorExternalId.ShouldBe("51"); + opened.Origin.IsFork.ShouldBeFalse(); + } + + [Fact] + public void A_code_push_carries_the_same_origin_and_names_who_pushed() + { + // On an update that moved the head, user is whoever pushed — the one GitLab delivery where the actor IS the + // person who made the code what it is. + var synced = Normalize(Body("update", sourceProjectId: 16, visibilityLevel: 10, oldrev: "\"abc\"")); + + synced.Origin.ShouldBe(new PullRequestOrigin { AuthorExternalId = "51", AuthorAssociation = PullRequestAuthorAssociation.Unknown, IsFork = true, HeadRepositoryFullName = "evil/api", RepositoryVisibility = RepositoryVisibility.Internal, PusherExternalId = "7" }); + ((IPullRequestOriginEvent)synced).HeadSha.ShouldBe("def"); + } + + [Theory] + [InlineData("open")] + [InlineData("reopen")] + public void An_open_names_its_head_commit_and_no_pusher(string action) + { + var opened = Normalize(Body(action, sourceProjectId: 16, visibilityLevel: 20)); + + opened.HeadSha.ShouldBe("def"); + opened.Origin.PusherExternalId.ShouldBeNull("user on an open or reopen is whoever opened it, not someone who pushed code"); + } + + [Theory] + [InlineData(0, RepositoryVisibility.Private)] + [InlineData(10, RepositoryVisibility.Internal)] + [InlineData(20, RepositoryVisibility.Public)] + [InlineData(99, null)] + public void Every_gitlab_visibility_level_maps_to_one_visibility(int level, RepositoryVisibility? expected) + { + Normalize(Body("open", sourceProjectId: 15, visibilityLevel: level)).Origin.RepositoryVisibility.ShouldBe(expected); + } + + [Fact] + public void A_payload_without_origin_fields_knows_nothing_and_is_not_called_a_fork() + { + const string body = """{"object_kind":"merge_request","user":{"id":1,"username":"alice"},"object_attributes":{"id":99,"iid":5,"title":"t","source_branch":"f","target_branch":"main","action":"open","url":"https://x"}}"""; + + var opened = Normalize(body); + + opened.Origin.ShouldBe(new PullRequestOrigin()); + } + + private T Normalize(string body) where T : class + { + var headers = new Dictionary { ["X-Gitlab-Event"] = "Merge Request Hook", ["X-Gitlab-Event-UUID"] = "u-1" }; + + return _normalizer.Normalize(_repositoryId, body, headers).ShouldBeOfType(); + } + + private static string Body(string action, int sourceProjectId, int visibilityLevel, string oldrev = "null") => + $$""" + {"object_kind":"merge_request","user":{"id":7,"username":"maintainer"}, + "project":{"id":15,"path_with_namespace":"acme/api","visibility_level":{{visibilityLevel}}}, + "object_attributes":{"id":99,"iid":5,"author_id":51,"source_project_id":{{sourceProjectId}},"target_project_id":15, + "source":{"path_with_namespace":"{{(sourceProjectId == 15 ? "acme/api" : "evil/api")}}"},"target":{"path_with_namespace":"acme/api"}, + "title":"t","source_branch":"main","target_branch":"main","action":"{{action}}","url":"https://x","oldrev":{{oldrev}},"last_commit":{"id":"def"} }, + "labels":[]} + """; +} diff --git a/backend/tests/CodeSpace.UnitTests/Webhooks/IngressRefusalFailureTests.cs b/backend/tests/CodeSpace.UnitTests/Webhooks/IngressRefusalFailureTests.cs new file mode 100644 index 000000000..44894caa4 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Webhooks/IngressRefusalFailureTests.cs @@ -0,0 +1,32 @@ +using CodeSpace.Core.Services.Webhooks.Exceptions; +using CodeSpace.Core.Services.Workflows.RunSources.Admission.Exceptions; +using CodeSpace.Messages.Failures; +using Shouldly; + +namespace CodeSpace.UnitTests.Webhooks; + +/// +/// What each ingress refusal means to a caller that sees only an . A delivery missing the id its +/// provider always sends is malformed; an author a members-only trigger does not admit is a known identity that is not +/// enough; a debounced pull request is a spent window that the same event clears later, untouched. +/// +[Trait("Category", "Unit")] +public class IngressRefusalFailureTests +{ + public static TheoryData Refusals => new() + { + { new WebhookDeliveryUnidentifiedException(Guid.NewGuid(), "X-GitHub-Delivery"), FailureKind.Invalid, FailureCodes.InvalidRequest }, + { new PullRequestAuthorRefusedException("pull request #7 was written by an author whose standing is 'none'", "audit-key"), FailureKind.Forbidden, FailureCodes.Forbidden }, + { new PullRequestTriggerDebouncedException(7, "head-1", TimeSpan.FromSeconds(60), "audit-key"), FailureKind.Exhausted, FailureCodes.RateLimited }, + }; + + [Theory] + [MemberData(nameof(Refusals))] + public void Each_refusal_says_what_a_caller_should_do_about_it(Exception refusal, FailureKind kind, string code) + { + var failure = refusal.ShouldBeAssignableTo().ShouldNotBeNull(); + + failure.Kind.ShouldBe(kind); + failure.Code.ShouldBe(code); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Webhooks/RejectedDeliveryHeadersTests.cs b/backend/tests/CodeSpace.UnitTests/Webhooks/RejectedDeliveryHeadersTests.cs new file mode 100644 index 000000000..320fba3a2 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Webhooks/RejectedDeliveryHeadersTests.cs @@ -0,0 +1,63 @@ +using System.Text.Json; +using CodeSpace.Core.Services.Webhooks; +using Shouldly; + +namespace CodeSpace.UnitTests.Webhooks; + +/// +/// What a refused delivery's headers cost to keep. The audit row is written before anything is authenticated, so whoever +/// posts decides the header names and the safe-listed values: sixty forged posts with a two-thousand-character header +/// name stored 135 KB. The row keeps a bounded number of headers, each name and value bounded, and keeps the provider's +/// own headers first so the cap never drops the ones an operator matches against the provider's delivery log. +/// +[Trait("Category", "Unit")] +public class RejectedDeliveryHeadersTests +{ + [Fact] + public void Secret_values_are_redacted_and_safe_values_kept() + { + var stored = Read(RejectedDeliveryHeaders.Serialize(new Dictionary { ["X-Hub-Signature-256"] = "sha256=abc", ["X-GitHub-Event"] = "push", ["User-Agent"] = "GitHub-Hookshot/1" })); + + stored["X-Hub-Signature-256"].ShouldBe("[REDACTED]"); + stored["X-GitHub-Event"].ShouldBe("push"); + stored["User-Agent"].ShouldBe("GitHub-Hookshot/1"); + } + + [Fact] + public void A_long_header_name_and_a_long_safe_value_are_truncated() + { + var name = new string('x', 2000); + var stored = Read(RejectedDeliveryHeaders.Serialize(new Dictionary { [name] = "v", ["User-Agent"] = new string('<', 5000) })); + + stored.Keys.ShouldAllBe(k => k.Length <= RejectedDeliveryHeaders.MaxNameLength + 1); + stored["User-Agent"].Length.ShouldBe(RejectedDeliveryHeaders.MaxValueLength + 1); + stored["User-Agent"].ShouldEndWith("…"); + } + + [Fact] + public void Past_the_cap_the_rest_are_counted_not_stored_and_the_providers_own_headers_survive() + { + var headers = Enumerable.Range(0, 500).ToDictionary(i => $"X-Junk-{i}", _ => "v"); + headers["X-GitHub-Delivery"] = "d-1"; + headers["X-GitHub-Event"] = "pull_request"; + + var json = RejectedDeliveryHeaders.Serialize(headers); + var stored = Read(json); + + stored.Count.ShouldBe(RejectedDeliveryHeaders.MaxHeaders + 1, "the cap, plus one entry saying how many were left out"); + stored["X-GitHub-Delivery"].ShouldBe("d-1"); + stored["X-GitHub-Event"].ShouldBe("pull_request"); + stored[RejectedDeliveryHeaders.OmittedKey].ShouldBe($"{502 - RejectedDeliveryHeaders.MaxHeaders} more headers not kept"); + json.Length.ShouldBeLessThan(4096, "a forged delivery's audit row is bounded whatever was posted"); + } + + [Fact] + public void The_caps_are_pinned() + { + RejectedDeliveryHeaders.MaxHeaders.ShouldBe(40); + RejectedDeliveryHeaders.MaxNameLength.ShouldBe(64); + RejectedDeliveryHeaders.MaxValueLength.ShouldBe(256); + } + + private static Dictionary Read(string json) => JsonSerializer.Deserialize>(json)!; +} diff --git a/backend/tests/CodeSpace.UnitTests/Webhooks/WebhookIngestionKeysTests.cs b/backend/tests/CodeSpace.UnitTests/Webhooks/WebhookIngestionKeysTests.cs new file mode 100644 index 000000000..0e37bba82 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Webhooks/WebhookIngestionKeysTests.cs @@ -0,0 +1,62 @@ +using CodeSpace.Core.Services.Webhooks; +using CodeSpace.Core.Services.Workflows.RunSources.Admission; +using CodeSpace.Messages.Constants; +using Shouldly; + +namespace CodeSpace.UnitTests.Webhooks; + +/// +/// The keys the unique index and the claim table decide on. Each one is what makes a flood collapse: a refusal row per +/// (hook, reason) per day, one accepted body per hook, one run per (activation, pull request, head) per debounce window. +/// +[Trait("Category", "Unit")] +public class WebhookIngestionKeysTests +{ + private static readonly Guid Hook = Guid.NewGuid(); + + [Fact] + public void A_refusal_collapses_per_hook_and_reason_within_a_day() + { + var morning = new DateTimeOffset(2026, 10, 8, 1, 0, 0, TimeSpan.Zero); + var evening = morning.AddHours(20); + + WebhookIngestionService.BuildRefusalWindowKey(Hook, WorkflowRunRequestRejectionReasons.SignatureInvalid, evening).ShouldBe(WebhookIngestionService.BuildRefusalWindowKey(Hook, WorkflowRunRequestRejectionReasons.SignatureInvalid, morning)); + WebhookIngestionService.BuildRefusalWindowKey(Hook, WorkflowRunRequestRejectionReasons.SignatureInvalid, morning.AddDays(1)).ShouldNotBe(WebhookIngestionService.BuildRefusalWindowKey(Hook, WorkflowRunRequestRejectionReasons.SignatureInvalid, morning)); + WebhookIngestionService.BuildRefusalWindowKey(Hook, WorkflowRunRequestRejectionReasons.WebhookInactive, morning).ShouldNotBe(WebhookIngestionService.BuildRefusalWindowKey(Hook, WorkflowRunRequestRejectionReasons.SignatureInvalid, morning)); + WebhookIngestionService.BuildRefusalWindowKey(Guid.NewGuid(), WorkflowRunRequestRejectionReasons.SignatureInvalid, morning).ShouldNotBe(WebhookIngestionService.BuildRefusalWindowKey(Hook, WorkflowRunRequestRejectionReasons.SignatureInvalid, morning)); + } + + [Fact] + public void A_body_is_claimed_per_hook_by_its_hash() + { + WebhookIngestionService.BuildBodyClaimKey(Hook, """{"a":1}""").ShouldBe(WebhookIngestionService.BuildBodyClaimKey(Hook, """{"a":1}""")); + WebhookIngestionService.BuildBodyClaimKey(Hook, """{"a":1}""").ShouldNotBe(WebhookIngestionService.BuildBodyClaimKey(Hook, """{"a":2}""")); + WebhookIngestionService.BuildBodyClaimKey(Guid.NewGuid(), """{"a":1}""").ShouldNotBe(WebhookIngestionService.BuildBodyClaimKey(Hook, """{"a":1}""")); + WebhookIngestionService.BuildBodyClaimKey(Hook, new string('x', 1_000_000)).Length.ShouldBeLessThan(120, "the key is a digest, never the body"); + } + + [Fact] + public void A_pull_request_is_debounced_per_activation_number_and_head() + { + // The head is in the key: a reopen loop re-sends a head that already ran, while a push moves the head and its + // commits must get their own run however soon after the last. + var activation = Guid.NewGuid(); + var repository = Guid.NewGuid(); + + PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 7, "sha-1").ShouldBe(PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 7, "sha-1")); + PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 7, "sha-2").ShouldNotBe(PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 7, "sha-1")); + PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 8, "sha-1").ShouldNotBe(PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 7, "sha-1")); + PullRequestTriggerAdmission.BuildDebounceKey(Guid.NewGuid(), repository, 7, "sha-1").ShouldNotBe(PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 7, "sha-1")); + PullRequestTriggerAdmission.BuildDebounceKey(activation, Guid.NewGuid(), 7, "sha-1").ShouldNotBe(PullRequestTriggerAdmission.BuildDebounceKey(activation, repository, 7, "sha-1")); + } + + [Fact] + public void The_windows_are_pinned() + { + // Committed values, changed by a PR that says why. The debounce is short so a reopen loop collapses while a reopen + // a minute later still runs; the replay window outlives GitHub's own redelivery window. + PullRequestTriggerAdmission.DebounceWindow.ShouldBe(TimeSpan.FromSeconds(60)); + WebhookIngestionService.ReplayWindow.ShouldBe(TimeSpan.FromDays(7)); + WebhookIngestionService.RefusalAuditWindow.ShouldBe(TimeSpan.FromDays(1)); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/NodeManifestContractTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/NodeManifestContractTests.cs index 52d58d15a..97b97ed88 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/NodeManifestContractTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/NodeManifestContractTests.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using System.Text.Json.Nodes; using CodeSpace.Core.Services.Workflows.Nodes; using CodeSpace.Core.Services.Workflows.Nodes.Builtin; using CodeSpace.Core.Services.Workflows.Runtime; @@ -360,21 +361,25 @@ public void Synchronous_nodes_declare_CanSuspend_false() new HttpRequestNode(null!).Manifest.CanSuspend.ShouldBeFalse("http.request runs synchronously (side-effecting but not parking)"); } - // PR-trigger schema dedup: every PR trigger must use the SAME serialised ConfigSchema JSON string. - // If any node drifts (e.g. someone copy-edits one description and forgets the others), this - // test fails — preventing a subtle activation-model regression where one trigger has different - // filter semantics from the rest. + // PR-trigger schema dedup: the triggers an outsider can cause (opened, updated) share ONE ConfigSchema, and it is the + // merged trigger's repositories filter plus the authors filter — so the repository/label semantics cannot drift between + // any of the three, and only the triggers an outsider can reach offer a choice about outsiders. [Fact] - public void Pr_trigger_nodes_share_identical_ConfigSchema() + public void Pr_trigger_nodes_share_one_repositories_filter_and_only_outsider_reachable_ones_filter_authors() { - var opened = new TriggerPrOpenedNode().Manifest.ConfigSchema.GetRawText(); - var updated = new TriggerPrUpdatedNode().Manifest.ConfigSchema.GetRawText(); - var merged = new TriggerPrMergedNode().Manifest.ConfigSchema.GetRawText(); - updated.ShouldBe(opened, - "trigger.pr.opened and trigger.pr.updated must use the same ConfigSchema (PrTriggerSchemas.RepositoriesConfigSchemaJson); " + - "drift in repo-filter semantics between the two triggers is a silent activation regression"); - merged.ShouldBe(opened, - "trigger.pr.merged must use the same ConfigSchema (PrTriggerSchemas.RepositoriesConfigSchemaJson) as the other PR triggers; " + - "drift in repo-filter semantics is a silent activation regression"); + var openedRaw = new TriggerPrOpenedNode().Manifest.ConfigSchema.GetRawText(); + var opened = JsonNode.Parse(openedRaw)!.AsObject(); + var merged = JsonNode.Parse(new TriggerPrMergedNode().Manifest.ConfigSchema.GetRawText())!.AsObject(); + + new TriggerPrUpdatedNode().Manifest.ConfigSchema.GetRawText().ShouldBe(openedRaw, + "trigger.pr.opened and trigger.pr.updated must use the same ConfigSchema; drift between the two is a silent activation regression"); + + var authors = opened["properties"]!["authors"]!.AsObject(); + authors["enum"]!.AsArray().Select(v => v!.GetValue()).ShouldBe(new[] { "any", "members" }); + authors.ContainsKey("default").ShouldBeFalse("no default: an activation naming none takes its repository's default (members on public or internal, anyone on private), and a schema default would be written into every config the editor saves"); + + opened["properties"]!.AsObject().Remove("authors"); + merged.ToJsonString().ShouldBe(opened.ToJsonString(), + "trigger.pr.merged must use the same repositories filter as the other PR triggers, and no authors filter: a merge needs a member's hand"); } } diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/PullRequestTriggerAuthorsTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/PullRequestTriggerAuthorsTests.cs new file mode 100644 index 000000000..26e79f320 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/PullRequestTriggerAuthorsTests.cs @@ -0,0 +1,112 @@ +using System.Text.Json; +using CodeSpace.Core.Services.Workflows.RunSources.Admission; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Events.PullRequest; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// Who may start a run with a pull request. An activation names authors: any | members; one that names neither — +/// every activation saved before this existed — takes the default for the repository the PR targets: members only when +/// the provider says it is public or internal (anyone who can read either can fork it and open a PR), anyone when it +/// says it is private, and, when the delivery does not say, members only for a PR from a fork. So an existing activation +/// keeps admitting everyone only on a private repository. Under members, new commits must also have been pushed by the +/// author or by a member: a member's PR whose head lives in an outsider's fork is the outsider's code from then on. +/// +[Trait("Category", "Unit")] +public class PullRequestTriggerAuthorsTests +{ + [Theory] + [InlineData(RepositoryVisibility.Public, false, PullRequestTriggerAuthors.Members)] + [InlineData(RepositoryVisibility.Public, true, PullRequestTriggerAuthors.Members)] + [InlineData(RepositoryVisibility.Private, false, PullRequestTriggerAuthors.Any)] + [InlineData(RepositoryVisibility.Private, true, PullRequestTriggerAuthors.Any)] + [InlineData(RepositoryVisibility.Internal, false, PullRequestTriggerAuthors.Members)] + [InlineData(RepositoryVisibility.Internal, true, PullRequestTriggerAuthors.Members)] + [InlineData(null, true, PullRequestTriggerAuthors.Members)] + [InlineData(null, false, PullRequestTriggerAuthors.Any)] + public void An_activation_naming_no_authors_takes_the_repositorys_default(RepositoryVisibility? visibility, bool isFork, string expected) + { + var origin = new PullRequestOrigin { RepositoryVisibility = visibility, IsFork = isFork }; + + PullRequestTriggerAuthors.Required(Config("{}"), origin).ShouldBe(expected); + } + + [Theory] + [InlineData("""{"authors":"any"}""", PullRequestTriggerAuthors.Any)] + [InlineData("""{"authors":"members"}""", PullRequestTriggerAuthors.Members)] + [InlineData("""{"authors":"everyone"}""", PullRequestTriggerAuthors.Members)] + [InlineData("""{"authors":7}""", PullRequestTriggerAuthors.Members)] + [InlineData("""{"authors":null}""", PullRequestTriggerAuthors.Any)] + public void An_activation_naming_authors_gets_what_it_named_and_a_value_it_cannot_mean_admits_only_members(string configJson, string expected) + { + // Named on a PRIVATE repository so the default would be "any": an explicit "members" and an unreadable value + // both have to win over it. A null is the editor's cleared field, which is "no choice made". + var origin = new PullRequestOrigin { RepositoryVisibility = RepositoryVisibility.Private }; + + PullRequestTriggerAuthors.Required(Config(configJson), origin).ShouldBe(expected); + } + + [Fact] + public void An_explicit_any_admits_an_outsider_on_a_public_repository() + { + var origin = new PullRequestOrigin { RepositoryVisibility = RepositoryVisibility.Public, IsFork = true, AuthorAssociation = PullRequestAuthorAssociation.None }; + + PullRequestTriggerAuthors.Admits(PullRequestTriggerAuthors.Required(Config("""{"authors":"any"}"""), origin), origin.AuthorAssociation).ShouldBeTrue(); + } + + [Theory] + [InlineData(PullRequestAuthorAssociation.Member, true)] + [InlineData(PullRequestAuthorAssociation.Contributor, false)] + [InlineData(PullRequestAuthorAssociation.None, false)] + [InlineData(PullRequestAuthorAssociation.Unknown, false)] + public void Members_admits_only_a_member_and_never_an_unknown(PullRequestAuthorAssociation association, bool admitted) + { + PullRequestTriggerAuthors.Admits(PullRequestTriggerAuthors.Members, association).ShouldBe(admitted); + } + + [Theory] + [InlineData(PullRequestAuthorAssociation.Member)] + [InlineData(PullRequestAuthorAssociation.None)] + [InlineData(PullRequestAuthorAssociation.Unknown)] + public void Any_admits_everyone(PullRequestAuthorAssociation association) + { + PullRequestTriggerAuthors.Admits(PullRequestTriggerAuthors.Any, association).ShouldBeTrue(); + } + + [Theory] + [InlineData(null, true, PullRequestAuthorAssociation.Unknown, true)] // an open or reopen: nobody pushed anything + [InlineData("9", true, PullRequestAuthorAssociation.Unknown, true)] // the author pushed; their standing was already decided + [InlineData("66", false, PullRequestAuthorAssociation.Unknown, true)] // the head lives in the target repository: pushing there takes a role + [InlineData("66", true, PullRequestAuthorAssociation.Member, true)] // someone else pushed to a fork head, and is a member + [InlineData("66", true, PullRequestAuthorAssociation.Contributor, false)] + [InlineData("66", true, PullRequestAuthorAssociation.None, false)] + [InlineData("66", true, PullRequestAuthorAssociation.Unknown, false)] // GitHub names the pusher but not their standing + public void Members_admits_new_commits_only_when_the_author_or_a_member_pushed_them(string? pusher, bool isFork, PullRequestAuthorAssociation pusherAssociation, bool admitted) + { + var origin = new PullRequestOrigin { AuthorExternalId = "9", AuthorAssociation = PullRequestAuthorAssociation.Member, IsFork = isFork, PusherExternalId = pusher, PusherAssociation = pusherAssociation }; + + PullRequestTriggerAuthors.AdmitsPusher(PullRequestTriggerAuthors.Members, origin).ShouldBe(admitted); + } + + [Fact] + public void Any_admits_whoever_pushed() + { + var origin = new PullRequestOrigin { AuthorExternalId = "9", IsFork = true, PusherExternalId = "66", PusherAssociation = PullRequestAuthorAssociation.None }; + + PullRequestTriggerAuthors.AdmitsPusher(PullRequestTriggerAuthors.Any, origin).ShouldBeTrue(); + } + + [Fact] + public void The_config_key_and_values_are_pinned() + { + // Stored in workflow_activation.config_json and in every saved definition's trigger node. A rename silently + // turns every explicit choice into the default. + PullRequestTriggerAuthors.ConfigKey.ShouldBe("authors"); + PullRequestTriggerAuthors.Any.ShouldBe("any"); + PullRequestTriggerAuthors.Members.ShouldBe("members"); + } + + private static JsonElement Config(string json) => JsonDocument.Parse(json).RootElement; +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/RunSourceDispatcherTenancyTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/RunSourceDispatcherTenancyTests.cs index 9390e1af5..3f7f9f36d 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/RunSourceDispatcherTenancyTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/RunSourceDispatcherTenancyTests.cs @@ -5,10 +5,13 @@ using CodeSpace.Core.Services.Completion.Exceptions; using CodeSpace.Core.Services.Workflows.Dispatch; using CodeSpace.Core.Services.Workflows.RunSources; +using CodeSpace.Core.Services.Workflows.RunSources.Admission; +using CodeSpace.Core.Services.Workflows.RunSources.Admission.Exceptions; using CodeSpace.Core.Services.Workflows.RunSources.Matchers; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Events; using CodeSpace.Messages.Events.PullRequest; +using System.Text.Json; using CodeSpace.UnitTests.Infrastructure; using Microsoft.Extensions.Logging.Abstractions; using Shouldly; @@ -91,6 +94,55 @@ public async Task A_refusing_activation_is_audited_and_its_sibling_still_starts( world.Auditor.NoMatches.ShouldBeEmpty(customMessage: "a refusal is its own reason — it must not also read as 'nothing was listening'"); } + [Theory] + [InlineData(PullRequestRefusal.AuthorNotMember, WorkflowRunRequestRejectionReasons.AuthorNotMember)] + [InlineData(PullRequestRefusal.Debounced, WorkflowRunRequestRejectionReasons.PullRequestDebounced)] + public async Task A_pull_request_admission_refusal_costs_its_own_run_and_collapses_on_the_refusals_own_key(PullRequestRefusal refusal, string expectedReason) + { + using var world = new World(); + var refusing = world.Workflow(world.EventTeamId, PrOpened, "{}"); + var sibling = world.Workflow(world.EventTeamId, PrOpened, """{"authors":"any"}"""); + await world.SaveAsync(); + world.Admission.Refuse(refusing, refusal); + + await world.DispatchAsync(world.OpenedEvent()); + + world.Starter.Started.Select(e => e.WorkflowId).ShouldBe(new[] { sibling }, customMessage: "an author or debounce refusal belongs to one activation"); + var audit = world.Auditor.Rejections.ShouldHaveSingleItem(); + audit.Reason.ShouldBe(expectedReason); + audit.DedupKey.ShouldBe(RecordingAdmission.AuditKey, customMessage: "a reopen loop is a stream of genuinely signed deliveries — the row collapses per PR, not per delivery"); + world.Auditor.NoMatches.ShouldBeEmpty(); + } + + [Fact] + public async Task Admission_is_asked_only_after_the_matcher_said_yes_and_sees_the_activations_own_config() + { + using var world = new World(); + world.Workflow(world.EventTeamId, PrOpened, $$"""{"repositories":[{"repositoryId":"{{Guid.NewGuid()}}"}]}"""); + var matching = world.Workflow(world.EventTeamId, PrOpened, """{"authors":"members"}"""); + await world.SaveAsync(); + + await world.DispatchAsync(world.OpenedEvent()); + + world.Admission.Asked.ShouldBe(new (Guid, string?)[] { (matching, "members") }); + } + + [Fact] + public async Task Activations_are_visited_in_one_order_whatever_order_they_were_stored_in() + { + // Each visit can take a row lock — the PR debounce claim — held until the delivery commits. Two concurrent + // deliveries for one PR that visited the same activations in different orders could each hold the lock the + // other waits on. Visiting by id gives every delivery the same order. + using var world = new World(); + var ids = new[] { 3, 1, 2 }.Select(n => new Guid($"00000000-0000-0000-0000-00000000000{n}")).ToList(); + foreach (var id in ids) world.Workflow(world.EventTeamId, PrOpened, "{}", activationId: id); + await world.SaveAsync(); + + await world.DispatchAsync(world.OpenedEvent()); + + world.Admission.ActivationIds.ShouldBe(ids.OrderBy(id => id).ToList(), customMessage: "check the ORDER BY in RunSourceDispatcher.LoadActiveActivationsAsync"); + } + [Fact] public async Task An_infrastructure_failure_is_not_a_refusal_and_propagates() { @@ -107,6 +159,8 @@ public async Task An_infrastructure_failure_is_not_a_refusal_and_propagates() public enum Refusal { CompletionAdmission, Authority } + public enum PullRequestRefusal { AuthorNotMember, Debounced } + /// One in-memory database holding the event's repository in its team, plus the fakes the dispatcher talks to. private sealed class World : IDisposable { @@ -116,19 +170,20 @@ private sealed class World : IDisposable public Guid RepositoryId { get; } = Guid.NewGuid(); public RecordingStarter Starter { get; } = new(); public RecordingAuditor Auditor { get; } = new(); + public RecordingAdmission Admission { get; } = new(); public World(bool repositoryDeleted = false) { _db.Repository.Add(new Repository { Id = RepositoryId, TeamId = EventTeamId, ProviderInstanceId = Guid.NewGuid(), ExternalId = "1", NamespacePath = "acme", Name = "api", FullPath = "acme/api", WebUrl = "https://x", DeletedDate = repositoryDeleted ? DateTimeOffset.UtcNow : null }); } - public Guid Workflow(Guid teamId, string typeKey, string configJson) + public Guid Workflow(Guid teamId, string typeKey, string configJson, Guid? activationId = null) { var workflowId = Guid.NewGuid(); var publisher = Guid.NewGuid(); _db.Workflow.Add(new Workflow { Id = workflowId, TeamId = teamId, Slug = $"wf-{workflowId:N}", Name = "wf", DefinitionJson = "{}", LatestVersion = 1, Enabled = true, CreatedBy = publisher, LastModifiedBy = publisher }); - _db.WorkflowActivation.Add(new WorkflowActivation { Id = Guid.NewGuid(), WorkflowId = workflowId, TypeKey = typeKey, ConfigJson = configJson, Enabled = true, CreatedBy = publisher, LastModifiedBy = publisher }); + _db.WorkflowActivation.Add(new WorkflowActivation { Id = activationId ?? Guid.NewGuid(), WorkflowId = workflowId, TypeKey = typeKey, ConfigJson = configJson, Enabled = true, CreatedBy = publisher, LastModifiedBy = publisher }); return workflowId; } @@ -155,7 +210,7 @@ public Guid Workflow(Guid teamId, string typeKey, string configJson) public Task DispatchAsync(PullRequestOpenedEvent normalizedEvent) { var registry = new RunSourceMatcherRegistry(new IRunSourceMatcher[] { new PrOpenedMatcher(), new PrUpdatedMatcher(), new PrMergedMatcher(), new PushMatcher() }); - var dispatcher = new RunSourceDispatcher(_db, registry, Starter, new NoopRunDispatcher(), Auditor, new InlinePostCommitActions(), NullLogger.Instance); + var dispatcher = new RunSourceDispatcher(_db, registry, Starter, new NoopRunDispatcher(), Auditor, new InlinePostCommitActions(), Admission, NullLogger.Instance); return dispatcher.Handle(normalizedEvent, CancellationToken.None); } @@ -186,6 +241,35 @@ public Task StartAsync(RunSourceEnvelope envelope, CancellationToken cance } } + /// Admits everything unless told to refuse one workflow; records what it was asked, so a test can see the dispatcher asks AFTER matching. + private sealed class RecordingAdmission : IPullRequestTriggerAdmission + { + public const string AuditKey = "rejected:test:per-pr"; + + private readonly Dictionary _refusals = new(); + + public List<(Guid WorkflowId, string? Authors)> Asked { get; } = new(); + + /// The activations asked about, in the order the dispatcher visited them. + public List ActivationIds { get; } = new(); + + public void Refuse(Guid workflowId, PullRequestRefusal refusal) => _refusals[workflowId] = refusal; + + public Task EnsureAdmittedAsync(WorkflowActivation activation, NormalizedEvent normalizedEvent, JsonElement activationConfig, CancellationToken cancellationToken) + { + Asked.Add((activation.WorkflowId, activationConfig.TryGetProperty("authors", out var authors) ? authors.GetString() : null)); + ActivationIds.Add(activation.Id); + + if (!_refusals.TryGetValue(activation.WorkflowId, out var refusal)) return Task.CompletedTask; + + Exception error = refusal == PullRequestRefusal.Debounced + ? new PullRequestTriggerDebouncedException(42, "head-1", TimeSpan.FromSeconds(60), AuditKey) + : new PullRequestAuthorRefusedException("pull request #42 was written by an author whose standing is 'none'", AuditKey); + + return Task.FromException(error); + } + } + private sealed class RecordingAuditor : IIngestionAuditor { public List Rejections { get; } = new(); diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/TriggerMatcherTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/TriggerMatcherTests.cs index 15f6caef8..520253b1e 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/TriggerMatcherTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/TriggerMatcherTests.cs @@ -681,6 +681,44 @@ public void PrUpdated_OutputSchema_lists_exactly_the_keys_BuildPayload_emits() "See PrOpened_OutputSchema_lists_exactly_the_keys_BuildPayload_emits for the failure mode and fix recipe."); } + [Fact] + public void PrOpened_payload_says_who_wrote_the_pr_and_where_its_head_lives() + { + // A fork's head branch can be named "main" too: without these three a workflow cannot tell an outsider's PR + // from a member's, nor gate on it. + var ev = OpenedEvent(RepoA); + ev.Origin = new PullRequestOrigin { AuthorAssociation = CodeSpace.Messages.Enums.PullRequestAuthorAssociation.None, IsFork = true, HeadRepositoryFullName = "evil/repo" }; + + var payload = new PrOpenedMatcher().BuildPayload(ev); + + payload.GetProperty("authorAssociation").GetString().ShouldBe("none"); + payload.GetProperty("isFork").GetBoolean().ShouldBeTrue(); + payload.GetProperty("headRepositoryFullName").GetString().ShouldBe("evil/repo"); + } + + [Fact] + public void PrUpdated_payload_says_who_wrote_the_pr_and_where_its_head_lives() + { + var ev = new PullRequestSynchronizedEvent { RepositoryId = RepoA, ProviderEventId = "1", OccurredAt = DateTimeOffset.UtcNow, ExternalPullRequestId = "1", Number = 42, PreviousHeadSha = "a", NewHeadSha = "b" }; + ev.Origin = new PullRequestOrigin { AuthorAssociation = CodeSpace.Messages.Enums.PullRequestAuthorAssociation.Member, IsFork = false, HeadRepositoryFullName = "acme/api" }; + + var payload = new PrUpdatedMatcher().BuildPayload(ev); + + payload.GetProperty("authorAssociation").GetString().ShouldBe("member"); + payload.GetProperty("isFork").GetBoolean().ShouldBeFalse(); + payload.GetProperty("headRepositoryFullName").GetString().ShouldBe("acme/api"); + } + + [Fact] + public void An_unknown_origin_reads_unknown_and_not_a_fork() + { + var payload = new PrOpenedMatcher().BuildPayload(OpenedEvent(RepoA)); + + payload.GetProperty("authorAssociation").GetString().ShouldBe("unknown"); + payload.GetProperty("isFork").GetBoolean().ShouldBeFalse(); + payload.GetProperty("headRepositoryFullName").ValueKind.ShouldBe(JsonValueKind.Null); + } + /// /// Pulls the top-level properties object's keys from a JSON Schema, the /// same surface SchemaForm walks to render fields. Tolerates missing / diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowRunRequestRejectionReasonsTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowRunRequestRejectionReasonsTests.cs index bf8c65ed8..f307beb6c 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowRunRequestRejectionReasonsTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowRunRequestRejectionReasonsTests.cs @@ -21,6 +21,10 @@ public class WorkflowRunRequestRejectionReasonsTests [InlineData("webhook_retired", nameof(WorkflowRunRequestRejectionReasons.WebhookRetired))] [InlineData("completion_admission_refused", nameof(WorkflowRunRequestRejectionReasons.CompletionAdmissionRefused))] [InlineData("repository_outside_hook_owner", nameof(WorkflowRunRequestRejectionReasons.RepositoryOutsideHookOwner))] + [InlineData("delivery_id_missing", nameof(WorkflowRunRequestRejectionReasons.DeliveryIdMissing))] + [InlineData("delivery_replayed", nameof(WorkflowRunRequestRejectionReasons.DeliveryReplayed))] + [InlineData("author_not_member", nameof(WorkflowRunRequestRejectionReasons.AuthorNotMember))] + [InlineData("pull_request_debounced", nameof(WorkflowRunRequestRejectionReasons.PullRequestDebounced))] public void Rejection_reason_string_form_is_pinned(string expected, string constantName) { var actual = typeof(WorkflowRunRequestRejectionReasons).GetField(constantName)!.GetValue(null) as string; diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 3c439ba11..5bbe1e533 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -291,6 +291,7 @@ export interface RepositoryWebhookSecret { export type RejectionReason = | "signature_invalid" | "webhook_inactive" | "event_not_mapped" | "malformed_payload" | "no_matching_activation" | "repository_not_bound" | "webhook_retired" + | "delivery_id_missing" | "delivery_replayed" | "author_not_member" | "pull_request_debounced" | (string & {}); /** One delivery that arrived and was refused. Every field here was captured at rejection with secrets already stripped. */ diff --git a/frontend/src/components/repositories/RepositoryWebhooksPanel.test.tsx b/frontend/src/components/repositories/RepositoryWebhooksPanel.test.tsx index 8337b1558..78deaea60 100644 --- a/frontend/src/components/repositories/RepositoryWebhooksPanel.test.tsx +++ b/frontend/src/components/repositories/RepositoryWebhooksPanel.test.tsx @@ -544,4 +544,54 @@ describe("repository webhooks panel", () => { expect(toneOf("The hook was retired and is still sending")).toBe("bad"); expect(screen.getByText(/Remove the hook by hand at GitLab/)).toBeTruthy(); }); + + it("reads a pull request from outside the repository as the trigger's own filter, and names the setting that widens it", async () => { + // A members-only trigger doing its job — by default on a public or internal repository. Not a fault, but + // the one setting that changes it has to be named, or an operator who expected a run cannot get one. + renderPanel([hook()], { provider: "GitHub", refusals: [refusal({ reason: "author_not_member", detail: "Activation a1 workflow w1: pull request #7 was written by an author whose standing is 'none'", verificationResultJson: null })] }); + + const copy = rejectionCopy("author_not_member", "GitHub"); + await waitFor(() => expect(screen.getByText(copy.headline)).toBeTruthy()); + + expect(toneOf(copy.headline)).toBe("idle"); + expect(screen.getByText(/for new commits, pushed by the author or by someone with a role/)).toBeTruthy(); + expect(screen.getByText(/on a public or internal repository unless told otherwise/)).toBeTruthy(); + expect(screen.getByText(/set “Pull requests from” to Anyone/)).toBeTruthy(); + expect(screen.getByText(/At most one of these is recorded per pull request per day/)).toBeTruthy(); + expect(document.body.textContent).not.toContain("author_not_member"); + }); + + it("reads a debounced pull request as the same commit arriving again, and never as new commits held back", async () => { + renderPanel([hook()], { provider: "GitHub", refusals: [refusal({ reason: "pull_request_debounced", detail: "Activation a1 workflow w1: pull request #7 at 3f2c already started a run from this trigger in the last 60 seconds", verificationResultJson: null })] }); + + const copy = rejectionCopy("pull_request_debounced", "GitHub"); + await waitFor(() => expect(screen.getByText(copy.headline)).toBeTruthy()); + + expect(toneOf(copy.headline)).toBe("idle"); + expect(screen.getByText(/at the same head commit less than a minute earlier/)).toBeTruthy(); + expect(screen.getByText(/New commits always start their own run/)).toBeTruthy(); + expect(document.body.textContent).not.toContain("push again"); + }); + + it("reads a body replayed under a fresh delivery id as something the provider does not do, and points at the secret", async () => { + renderPanel([hook()], { provider: "GitHub", refusals: [refusal({ reason: "delivery_replayed", detail: "delivery attacker-2 to webhook 7f3a carries a signed body this hook already accepted under another delivery id", verificationResultJson: null })] }); + + const copy = rejectionCopy("delivery_replayed", "GitHub"); + await waitFor(() => expect(screen.getByText(copy.headline)).toBeTruthy()); + + expect(toneOf(copy.headline)).toBe("bad"); + expect(screen.getByText(/GitHub keeps a delivery's id when it redelivers/)).toBeTruthy(); + expect(screen.getByText(/rotate the hook's secret/)).toBeTruthy(); + }); + + it("reads a signed delivery with no delivery id as malformed for its provider, not as a signature fault", async () => { + renderPanel([hook()], { provider: "GitHub", refusals: [refusal({ reason: "delivery_id_missing", detail: "a signed delivery to webhook 7f3a carried no X-GitHub-Delivery", verificationResultJson: null })] }); + + const copy = rejectionCopy("delivery_id_missing", "GitHub"); + await waitFor(() => expect(screen.getByText(copy.headline)).toBeTruthy()); + + expect(toneOf(copy.headline)).toBe("bad"); + expect(screen.getByText(/GitHub sends an id with every delivery/)).toBeTruthy(); + expect(screen.queryByText("The signature did not match")).toBeNull(); + }); }); diff --git a/frontend/src/lib/webhookState.ts b/frontend/src/lib/webhookState.ts index daa9607ad..831027377 100644 --- a/frontend/src/lib/webhookState.ts +++ b/frontend/src/lib/webhookState.ts @@ -239,6 +239,34 @@ const REJECTION_COPY: Record RejectionCopy> = { headline: "The hook was retired and is still sending", remedy: `CodeSpace asked ${who} to delete this hook when the connection changed webhook scope, and could not. ${who} is still delivering to it and every delivery is discarded. Remove the hook by hand at ${who} — nothing here will run off it again.`, }), + // A signed delivery the provider would never send: the id it stamps on every delivery is missing. + // Either something in front of us strips headers or the request did not come from the provider. + delivery_id_missing: (who) => ({ + tone: "bad", + headline: "A signed delivery that carried no delivery id", + remedy: `${who} sends an id with every delivery and this one had none, so nothing it carried was acted on. Either something between ${who} and here strips request headers, or the delivery did not come from ${who} — if nothing in front of CodeSpace rewrites headers, rotate the hook's secret.`, + }), + // The provider's signature covers the body only, so a captured body stays valid. A redelivery keeps + // its id; the same body under a new one is somebody posting a captured delivery again. + delivery_replayed: (who) => ({ + tone: "bad", + headline: "A delivery that repeated one already accepted", + remedy: `Signed correctly, but its body is the same as a delivery ${who} already sent here under another delivery id, so it started nothing. ${who} keeps a delivery's id when it redelivers, so this was not a redelivery: something posted a captured delivery again. If that was not you, rotate the hook's secret.`, + }), + // The trigger's own filter at work — on a public or internal repository, by default. Not a fault, but + // the one setting that changes it is named, or an operator expecting a run has nowhere to go. + author_not_member: (who) => ({ + tone: "idle", + headline: "A pull request from someone without a role on the repository", + remedy: `Not a fault. This trigger starts runs only for pull requests written by someone with a role on the repository at ${who} — and, for new commits, pushed by the author or by someone with a role — which is what a trigger does on a public or internal repository unless told otherwise. To let anyone's pull request start it, set “Pull requests from” to Anyone on the trigger. At most one of these is recorded per pull request per day, however many arrive.`, + }), + // A head commit that already ran arriving again — a close/reopen loop. Never newer code: a push moves + // the head, and a new head always runs. + pull_request_debounced: () => ({ + tone: "idle", + headline: "A pull request that had just run at the same commit", + remedy: "Not a fault. The same trigger started a run for this pull request at the same head commit less than a minute earlier, so this event — a reopen, or the same commits arriving again — started nothing. New commits always start their own run. At most one of these is recorded per pull request and commit per day.", + }), // Deliberately the friendliest of the five. This is not a failure — it is the delivery arriving, // being verified, being understood, and finding that nothing asked for it. no_matching_activation: () => ({