diff --git a/backend/src/CodeSpace.Api/Controllers/WorkflowRunsController.cs b/backend/src/CodeSpace.Api/Controllers/WorkflowRunsController.cs index 1311ef970..16bc18da2 100644 --- a/backend/src/CodeSpace.Api/Controllers/WorkflowRunsController.cs +++ b/backend/src/CodeSpace.Api/Controllers/WorkflowRunsController.cs @@ -422,6 +422,19 @@ public async Task ReissueWait([FromRoute] Guid runId, [FromRoute] : Ok(new { outcome = outcome.ToString(), reissued = outcome == ReissueWaitOutcome.Reissued }); } + /// + /// The URL of a pending Callback wait — the one the external system POSTs to. Its token resumes the run without a + /// session, so no read projects it: this needs runs.control (a Viewer gets 403) and logs who took it. Team-scoped; + /// foreign, not pending or not a Callback → 404; a row that keeps only the token's digest → 409. Returns + /// { waitId, path }. + /// + [HttpPost("{runId:guid}/waits/{waitId:guid}/callback-url")] + public async Task RevealCallbackUrl([FromRoute] Guid runId, [FromRoute] Guid waitId, CancellationToken cancellationToken) + { + var result = await _mediator.Send(new RevealWorkflowCallbackUrlCommand { RunId = runId, WaitId = waitId }, cancellationToken).ConfigureAwait(false); + return Ok(result); + } + /// Continue a run IN PLACE (same run id, never a fork): a STRANDED Suspended run (the reconciler's re-dispatch twin), a terminal FAILURE run (re-run the halting node[s]), or a terminal CANCELLED run the operator stopped mid-flight (re-run the interrupted frontier). Team-scoped; foreign → 404. Returns { continued } — false when the state can't continue in place (Success/Running, or nothing incomplete to resume) so the caller falls back to replay / rerun. [HttpPost("{runId:guid}/continue")] public async Task Continue([FromRoute] Guid runId, CancellationToken cancellationToken) diff --git a/backend/src/CodeSpace.Api/Filters/GlobalExceptionFilter.cs b/backend/src/CodeSpace.Api/Filters/GlobalExceptionFilter.cs index 200c9e4f0..02b05f54f 100644 --- a/backend/src/CodeSpace.Api/Filters/GlobalExceptionFilter.cs +++ b/backend/src/CodeSpace.Api/Filters/GlobalExceptionFilter.cs @@ -1,3 +1,4 @@ +using CodeSpace.Api.Logging; using CodeSpace.Core.Failures; using CodeSpace.Messages.Failures; using Microsoft.AspNetCore.Mvc; @@ -69,7 +70,7 @@ private void LogIfNobodyElseDid(ExceptionContext context, FailureClassification context.Exception, "{Method} {Path} failed outside the request pipeline: {FailureKind}/{FailureCode}", context.HttpContext.Request.Method, - context.HttpContext.Request.Path.Value, + BearerRouteLogging.PathOf(context.HttpContext), failure.Kind, failure.Code); } diff --git a/backend/src/CodeSpace.Api/Logging/BearerRouteLogging.cs b/backend/src/CodeSpace.Api/Logging/BearerRouteLogging.cs new file mode 100644 index 000000000..242cf08c7 --- /dev/null +++ b/backend/src/CodeSpace.Api/Logging/BearerRouteLogging.cs @@ -0,0 +1,185 @@ +using CodeSpace.Core.Settings.Application; +using Microsoft.AspNetCore.Routing.Patterns; +using Serilog.Context; +using Serilog.Core; +using Serilog.Events; + +namespace CodeSpace.Api.Logging; + +/// +/// Keeps bearer tokens that travel in a URL PATH out of the logs. Invitation, password-reset and workflow-callback links +/// carry their token as a path segment (the URL shapes are issued and cannot change), and the request path is on every +/// line a request logs: in Serilog's completion line, and as RequestPath from the ASP.NET hosting scope on every +/// event logged while the request runs. Whoever could read the console or Seq could otherwise join a team, take over an +/// account or resume a run. +/// +/// For a matched route that binds a bearer parameter, both carry the route's TEMPLATE instead +/// (/api/invitations/{token}/accept), which still says which endpoint ran. Keyed on the parameter's NAME +/// () rather than a list of paths, so a new token route is covered by naming its parameter +/// token; an E2E test fails if a route binds a credential-looking parameter this does not recognise. +/// +/// The same token also arrives where no bearer route answers: a GET or HEAD on a callback URL (a link unfurler, a +/// person pasting it into a browser, an uptime ping), a wrong method on an invitation route, the SPA's own +/// /invite/… or /reset-password/… link opened on the API origin, and every line logged before routing has +/// chosen an endpoint (UseHttpsRedirection warns once per process without an https port). Those requests are answered by +/// the fallback, a method rejection or nothing yet, so the path's SHAPE decides: a path whose leading segments have the +/// shape of a bearer route up to its token is logged with that segment as {token} +/// (/api/workflows/callbacks/{token}). The shapes are the bearer routes in the host's endpoint table plus +/// . A request a non-bearer route answered logs exactly what it did before. +/// +/// What this cannot reach: lines the server writes outside the pipeline — ASP.NET's "Request starting …" before +/// this middleware runs, and its line for an exception that escapes every middleware (an action's exception never does; +/// the global filter handles it). The first is Information and the second Error under Microsoft.AspNetCore; the +/// shipped Serilog:MinimumLevel:Override:Microsoft = Warning keeps the first out, and a test pins that setting. +/// +public static class BearerRouteLogging +{ + /// Route parameter names whose value is a bearer credential. + public static readonly IReadOnlySet BearerParameters = new HashSet(StringComparer.OrdinalIgnoreCase) { "token" }; + + /// + /// The SPA's token routes — the links mints, and the prefixes the SPA's nginx masks. + /// This server never serves them, so no endpoint names them, but a link built against the API origin brings them here. + /// + public static readonly IReadOnlyList SpaBearerRoutes = [PublicBaseUrlSetting.InvitePath + "{token}", PublicBaseUrlSetting.PasswordResetPath + "{token}"]; + + private const string RequestPathProperty = "RequestPath"; + + /// The path to log for this request: a bearer route's template, else the path with any bearer segment masked, else the raw path. + public static string PathOf(HttpContext context) => TemplateOf(context) ?? context.Request.Path.Value ?? string.Empty; + + /// + /// RequestLoggingOptions.GetMessageTemplateProperties: the same four properties Serilog's default completion line + /// carries, with the bearer-safe path in place of a token's. is what Serilog would log. + /// + public static IEnumerable RequestCompletionProperties(HttpContext context, string requestPath, double elapsedMs, int statusCode) => + [ + new("RequestMethod", new ScalarValue(context.Request.Method)), + new(RequestPathProperty, new ScalarValue(TemplateOf(context) ?? requestPath)), + new("StatusCode", new ScalarValue(statusCode)), + new("Elapsed", new ScalarValue(elapsedMs)), + ]; + + /// + /// Middleware, FIRST in the pipeline: every event the request logs carries the bearer-safe path as RequestPath — + /// the completion line, everything downstream of routing, what routing itself logs the moment it matches, and what the + /// middleware ahead of routing logs. The hosting scope's raw value reaches an event through Serilog's logging provider, + /// which only adds a property that is absent; this enricher overwrites, so it wins in either order. + /// + public static IApplicationBuilder UseBearerRouteLogging(this IApplicationBuilder app) + { + var shapes = new BearerPathShapes(app.ApplicationServices.GetService()); + + return app.Use((context, next) => RewriteRequestPathAsync(context, next, shapes)); + } + + private static async Task RewriteRequestPathAsync(HttpContext context, RequestDelegate next, BearerPathShapes shapes) + { + context.Features.Set(shapes.Loaded()); + var route = new BearerRouteEnricher(context); + + try + { + using (LogContext.Push(route)) + await next(context).ConfigureAwait(false); + } + finally + { + route.Settle(); + } + } + + /// + /// A bearer route's template when one answered; null when any other route did, its literals accounting for the path; + /// otherwise — no endpoint yet, the catch-all fallback, a method rejection — the path with a bearer segment masked. + /// + private static string? TemplateOf(HttpContext context) => context.GetEndpoint() switch + { + RouteEndpoint endpoint when IsBearerRoute(endpoint.RoutePattern) => context.Request.PathBase.Value + "/" + endpoint.RoutePattern.RawText?.TrimStart('/'), + RouteEndpoint endpoint when !endpoint.RoutePattern.Parameters.Any(parameter => parameter.IsCatchAll) => null, + _ => context.Features.Get()?.Mask(context.Request.Path.Value) is { } masked ? context.Request.PathBase.Value + masked : null, + }; + + private static bool IsBearerRoute(RoutePattern pattern) => pattern.Parameters.Any(parameter => BearerParameters.Contains(parameter.Name)); + + /// + /// Every bearer route's shape, read once from the host's endpoint table on the first request (the table is complete only + /// once the host has started) — in the middleware, so a failure to read it fails that request rather than leaving a log + /// line unmasked. + /// + private sealed class BearerPathShapes + { + private readonly Lazy> _routes; + + public BearerPathShapes(EndpointDataSource? endpoints) => _routes = new(() => BearerRoutesOf(endpoints)); + + public BearerPathShapes Loaded() + { + _ = _routes.Value; + + return this; + } + + /// The path with every segment a bearer route's token would bind replaced by {name}; null when no bearer route has its shape. + public string? Mask(string? path) + { + var segments = (path ?? string.Empty).Split('/', StringSplitOptions.RemoveEmptyEntries); + var bearers = _routes.Value.Select(route => BearerSegmentOf(route, segments)).OfType<(int Index, string Name)>().ToList(); + + if (bearers.Count == 0) return null; + + foreach (var (index, name) in bearers) + segments[index] = "{" + name + "}"; + + return "/" + string.Join('/', segments); + } + + private static IReadOnlyList BearerRoutesOf(EndpointDataSource? endpoints) => + [.. SpaBearerRoutes.Select(RoutePatternFactory.Parse), .. endpoints?.Endpoints.OfType().Select(endpoint => endpoint.RoutePattern).Where(IsBearerRoute) ?? []]; + + /// + /// Walks the route's segments against the path's, case-insensitively as routing matches: a literal must equal its + /// segment, any other part matches anything, and reaching the bearer parameter names the segment it would bind. HTTP + /// method and constraints are ignored — a token in the path is a token whichever route would have refused it. + /// + private static (int Index, string Name)? BearerSegmentOf(RoutePattern route, string[] segments) + { + for (var i = 0; i < route.PathSegments.Count && i < segments.Length; i++) + { + var parts = route.PathSegments[i].Parts; + + if (parts is [RoutePatternParameterPart bearer] && BearerParameters.Contains(bearer.Name)) return (i, bearer.Name); + + if (parts is [RoutePatternLiteralPart literal] && !string.Equals(literal.Content, segments[i], StringComparison.OrdinalIgnoreCase)) return null; + } + + return null; + } + } + + /// + /// The request's bearer-safe path, read live while the request runs (routing matches it part-way through) and kept once + /// it ends, for work the request started that still logs — the server recycles the , so it is + /// never read after that. + /// + private sealed class BearerRouteEnricher : ILogEventEnricher + { + private volatile HttpContext? _request; + private volatile string? _settledTemplate; + + public BearerRouteEnricher(HttpContext request) { _request = request; } + + public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory) + { + var template = _request is { } request ? TemplateOf(request) : _settledTemplate; + + if (template != null) logEvent.AddOrUpdateProperty(propertyFactory.CreateProperty(RequestPathProperty, template)); + } + + public void Settle() + { + _settledTemplate = _request is { } request ? TemplateOf(request) : null; + _request = null; + } + } +} diff --git a/backend/src/CodeSpace.Api/Startup.cs b/backend/src/CodeSpace.Api/Startup.cs index c41aac430..0584d26ad 100644 --- a/backend/src/CodeSpace.Api/Startup.cs +++ b/backend/src/CodeSpace.Api/Startup.cs @@ -4,6 +4,7 @@ using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Services.Workflows.Llm; using CodeSpace.Api.Filters; +using CodeSpace.Api.Logging; using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.OutboundHttp; using CodeSpace.Core.Services.Identity; @@ -102,6 +103,10 @@ public void ConfigureServices(IServiceCollection services) public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { + // First, so every line a request logs — routing's own and the https redirect's ahead of it included, and requests + // only the fallback answers — names a bearer route by its template or masks the token's path segment, rather than + // carrying the invitation, reset or callback token in its path (see BearerRouteLogging). + app.UseBearerRouteLogging(); app.UseSerilogRequestLogging(opts => { opts.EnrichDiagnosticContext = (diag, ctx) => @@ -109,6 +114,8 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) var teamId = ctx.Request.Headers[HeaderCurrentTeam.HeaderName].ToString(); if (!string.IsNullOrEmpty(teamId)) diag.Set("TeamId", teamId); }; + // An invitation, reset or callback token in the path is logged as its route template, never verbatim. + opts.GetMessageTemplateProperties = BearerRouteLogging.RequestCompletionProperties; }); if (!env.IsDevelopment()) app.UseHttpsRedirection(); app.UseRouting(); diff --git a/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Workflows/RevealWorkflowCallbackUrlCommandHandler.cs b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Workflows/RevealWorkflowCallbackUrlCommandHandler.cs new file mode 100644 index 000000000..127c20331 --- /dev/null +++ b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Workflows/RevealWorkflowCallbackUrlCommandHandler.cs @@ -0,0 +1,22 @@ +using CodeSpace.Core.Services.Identity; +using CodeSpace.Core.Services.Workflows.Callbacks; +using CodeSpace.Messages.Commands.Workflows; +using CodeSpace.Messages.Dtos.Workflows; +using MediatR; + +namespace CodeSpace.Core.Handlers.CommandHandlers.Workflows; + +public sealed class RevealWorkflowCallbackUrlCommandHandler : IRequestHandler +{ + private readonly IWorkflowCallbackUrlService _service; + private readonly ICurrentTeam _currentTeam; + + public RevealWorkflowCallbackUrlCommandHandler(IWorkflowCallbackUrlService service, ICurrentTeam currentTeam) + { + _service = service; + _currentTeam = currentTeam; + } + + public async Task Handle(RevealWorkflowCallbackUrlCommand request, CancellationToken cancellationToken) => + await _service.RevealAsync(request.RunId, request.WaitId, _currentTeam.Id!.Value, cancellationToken).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0248_workflow_callback_token_digest.sql b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0248_workflow_callback_token_digest.sql new file mode 100644 index 000000000..c4599d3c7 --- /dev/null +++ b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0248_workflow_callback_token_digest.sql @@ -0,0 +1,52 @@ +-- 0248_workflow_callback_token_digest.sql +-- +-- A flow.wait_callback wait is resumed by an anonymous POST to /api/workflows/callbacks/{token}, so its token is a +-- bearer credential, and workflow_run_wait.token holds it in plaintext, readable by anyone with database or backup +-- access. This is the EXPAND step of moving it out of plaintext. It adds two columns and changes no existing value: +-- +-- token_digest the token's lowercase hex SHA-256, which this version looks a Callback wait up by (the way +-- invitation and reset tokens are kept). Filled below for every Callback row holding a minted token. +-- token_ciphertext the token AES-GCM encrypted (base64) under the deployment's variable master key +-- (Variables:MasterKey, read from CODESPACE_VARIABLE_MASTER_KEY), opened only by the reveal that needs +-- runs.control. That key is held by the environment and never by this database -- unlike the Data +-- Protection key ring, which sits in data_protection_keys. SQL cannot reach the key, so only rows this +-- version mints get one. +-- +-- token itself is left as it is, and this version keeps writing the minted token there, because a pod on the previous +-- build looks a callback up with `token = `. So: +-- * Any rollout order works -- API or worker first, pod by pod -- and a rollback to the previous build keeps every +-- callback URL resuming, including URLs this version issued. +-- * The digest is not a key anywhere: the previous build compares the presented value with token, which never holds a +-- digest, and this version compares the digest OF the presented value. (Both route only 32 characters since +-- a0eb90933; builds before it route any segment to that comparison.) +-- * A Callback row the previous build parks after this script ran has no digest and no ciphertext; this version still +-- resumes it by its token column and reveals it from there. +-- +-- CONTRACT STEP (a later release, not this script): only once no pod older than this version can run. It must first fill +-- token_digest where it is still NULL (rows previous-version pods parked after this script) and seal token_ciphertext in +-- the application, which holds the key; then set token to '' for Callback rows and stop writing it there. Never write +-- the digest, the projection placeholder, or any other value a callback route can carry into token: every build before +-- the contract step compares whatever is presented with that column. +-- +-- Both columns are NULLABLE with NO DEFAULT: catalog-only ADD COLUMNs. The UPDATE only fills a NULL digest for the minted +-- shape (32 lowercase hex), so running the script twice changes nothing. The partial index covers the one lookup that +-- reads the digest: a pending Callback wait by token_digest. + +ALTER TABLE workflow_run_wait ADD COLUMN IF NOT EXISTS token_digest TEXT NULL; +ALTER TABLE workflow_run_wait ADD COLUMN IF NOT EXISTS token_ciphertext TEXT NULL; + +COMMENT ON COLUMN workflow_run_wait.token_digest IS + 'A Callback wait''s token as lowercase hex SHA-256, which a resume looks the wait up by. NULL on other wait kinds, and on Callback rows a pre-0248 build parks.'; + +COMMENT ON COLUMN workflow_run_wait.token_ciphertext IS + 'A Callback wait''s token, AES-GCM encrypted (base64) under the deployment''s environment-held variable master key, opened only by the reveal that needs runs.control. NULL on other wait kinds, and on Callback rows minted before 0248 or parked by a pre-0248 build.'; + +UPDATE workflow_run_wait +SET token_digest = encode(sha256(convert_to(token, 'UTF8')), 'hex') +WHERE wait_kind = 'Callback' + AND token_digest IS NULL + AND token ~ '^[0-9a-f]{32}$'; + +CREATE INDEX IF NOT EXISTS idx_wrw_pending_callback_token_digest + ON workflow_run_wait (token_digest) + WHERE status = 'Pending' AND wait_kind = 'Callback'; diff --git a/backend/src/CodeSpace.Core/Persistence/Entities/WorkflowRunWait.cs b/backend/src/CodeSpace.Core/Persistence/Entities/WorkflowRunWait.cs index 4ddd73b49..6358287d5 100644 --- a/backend/src/CodeSpace.Core/Persistence/Entities/WorkflowRunWait.cs +++ b/backend/src/CodeSpace.Core/Persistence/Entities/WorkflowRunWait.cs @@ -25,9 +25,27 @@ public class WorkflowRunWait /// Why the run is parked + how it wakes. One of . public string WaitKind { get; set; } = default!; - /// Opaque correlation id an approval / callback signal presents to resolve this wait. + /// + /// Opaque correlation id a signal presents to resolve this wait. For a Callback wait — whose token is a bearer + /// credential for an anonymous route — it still holds the token as minted, because a pod on the previous build looks + /// the wait up by comparing this column with the presented token. A later release blanks it for Callback rows once no + /// such pod can run (see migration 0248); this version looks Callback waits up by first. + /// public string Token { get; set; } = default!; + /// + /// A Callback wait's token as lowercase hex SHA-256 (see WorkflowCallbackTokenSealer), which a resume looks + /// the wait up by. Null on every other kind, and on a Callback row a previous-version pod parked after migration 0248 ran. + /// + public string? TokenDigest { get; set; } + + /// + /// A Callback wait's token, encrypted under the deployment's environment-held variable master key (base64), for the + /// RunsControl-gated reveal. Null on every other kind, and on a Callback row minted before migration 0248 or parked by a + /// previous-version pod. + /// + public string? TokenCiphertext { get; set; } + /// For Timer waits — the instant the scheduled resume fires. Null for approval/callback. public DateTimeOffset? WakeAt { get; set; } diff --git a/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/WorkflowRunWaitConfiguration.cs b/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/WorkflowRunWaitConfiguration.cs index 1568cdf73..9c6f1201e 100644 --- a/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/WorkflowRunWaitConfiguration.cs +++ b/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/WorkflowRunWaitConfiguration.cs @@ -15,6 +15,8 @@ public void Configure(EntityTypeBuilder builder) builder.Property(w => w.WaitKind).HasMaxLength(24); // widened with 0054 to fit 'SupervisorDecision' (18) — keep in lockstep with the migration's VARCHAR(24) builder.Property(w => w.Status).HasMaxLength(16); builder.Property(w => w.Token).HasMaxLength(128); + builder.Property(w => w.TokenDigest).HasColumnName("token_digest"); + builder.Property(w => w.TokenCiphertext).HasColumnName("token_ciphertext"); builder.Property(w => w.NodeId).HasMaxLength(128); builder.Property(w => w.IterationKey).HasMaxLength(128); builder.Property(w => w.PayloadJson).HasColumnName("payload_jsonb").HasColumnType("jsonb"); diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/IWorkflowCallbackUrlService.cs b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/IWorkflowCallbackUrlService.cs new file mode 100644 index 000000000..b84edc940 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/IWorkflowCallbackUrlService.cs @@ -0,0 +1,14 @@ +using CodeSpace.Messages.Dtos.Workflows; + +namespace CodeSpace.Core.Services.Workflows.Callbacks; + +public interface IWorkflowCallbackUrlService +{ + /// + /// The URL of , a pending Callback wait on in + /// , logged as a reveal naming the caller. Throws when there + /// is no such pending callback wait in the team, and when its + /// row holds neither a ciphertext nor the token itself. + /// + Task RevealAsync(Guid runId, Guid waitId, Guid teamId, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackTokenSealer.cs b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackTokenSealer.cs new file mode 100644 index 000000000..dedd5ebed --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackTokenSealer.cs @@ -0,0 +1,67 @@ +using System.Security.Cryptography; +using System.Text; +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Services.Variables; +using CodeSpace.Messages.Workflows; + +namespace CodeSpace.Core.Services.Workflows.Callbacks; + +/// +/// How a callback wait's token is kept. The token is a bearer credential — the callback route is anonymous, so whoever +/// presents it resumes the run with a body of their choosing. Its row keeps it three ways: +/// +/// token_digest, its SHA-256, which a resume looks the wait up by (the way invitation and reset tokens are kept); +/// token_ciphertext, the token AES-GCM encrypted for the one reader allowed to see it again: the RunsControl-gated reveal; +/// token, the token as minted — kept for this release only, because a pod on the previous build looks the wait +/// up by comparing that column with the presented token. A later release blanks it once no such pod can run. +/// +/// +/// The encryption key is the deployment's variable master key (Variables:MasterKey), which the environment +/// holds and the database never does. Not Data Protection: its key ring is stored unencrypted in the same database +/// (data_protection_keys) under a public application name, so a database or backup reader could open anything it +/// protects. +/// +/// Older row shapes stay readable. A row minted before migration 0248, or parked by a previous-version pod, holds only +/// the token, in the shape produces (32 lowercase hex): a resume matches it in the token column, and a +/// reveal returns it as stored. A row whose token column a later release blanked holds only its digest and ciphertext. +/// +public sealed class WorkflowCallbackTokenSealer : ISingletonDependency +{ + private const int MintedLength = 32; + + private readonly IVariableValueEncryption _encryption; + + public WorkflowCallbackTokenSealer(IVariableValueEncryption encryption) => _encryption = encryption; + + /// A new token — the same 32-character shape the callback route admits and every earlier version minted — with the two other forms its row stores. + public SealedCallbackToken Mint() + { + var token = Guid.NewGuid().ToString("N"); + + return new SealedCallbackToken(token, Digest(token), Convert.ToBase64String(_encryption.Encrypt(token))); + } + + /// + /// The token a reveal hands out: the ciphertext opened when there is one, else the token column when it holds a minted + /// token, else null — a row whose token column was blanked and never sealed cannot give it back. A ciphertext sealed + /// under another master key throws . + /// + public string? Open(string storedToken, string? ciphertext) + { + if (ciphertext != null) return _encryption.Decrypt(Convert.FromBase64String(ciphertext)); + + return PlaintextFormOf(storedToken); + } + + /// Lowercase hex SHA-256 of the UTF-8 token — byte for byte what migration 0248 writes with encode(sha256(convert_to(token, 'UTF8')), 'hex'). + public static string Digest(string token) => Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(token))); + + /// + /// The value a row's token column holds for : the value itself when it has the minted shape, the + /// only shape any version writes there for a Callback, else null. So neither a digest (64 characters) nor a blanked + /// column is ever compared as a token. + /// + public static string? PlaintextFormOf(string value) => IsMintedShape(value) ? value : null; + + private static bool IsMintedShape(string value) => value.Length == MintedLength && value.All(char.IsAsciiHexDigitLower); +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackUrlService.cs b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackUrlService.cs new file mode 100644 index 000000000..86fc76cf7 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackUrlService.cs @@ -0,0 +1,59 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.Identity; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Dtos.Workflows; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Workflows.Callbacks; + +public sealed class WorkflowCallbackUrlService : IWorkflowCallbackUrlService, IScopedDependency +{ + /// The callback route on this API, ahead of the token. Pinned against WorkflowCallbacksController's route by a test, so the URL handed out is the one the route serves. + public const string RoutePrefix = "/api/workflows/callbacks/"; + + private readonly CodeSpaceDbContext _db; + private readonly WorkflowCallbackTokenSealer _sealer; + private readonly ICurrentUser _currentUser; + private readonly ILogger _logger; + + public WorkflowCallbackUrlService(CodeSpaceDbContext db, WorkflowCallbackTokenSealer sealer, ICurrentUser currentUser, ILogger logger) + { + _db = db; + _sealer = sealer; + _currentUser = currentUser; + _logger = logger; + } + + public async Task RevealAsync(Guid runId, Guid waitId, Guid teamId, CancellationToken cancellationToken) + { + var wait = await LoadPendingCallbackAsync(runId, waitId, teamId, cancellationToken).ConfigureAwait(false); + + var token = _sealer.Open(wait.Token, wait.TokenCiphertext) ?? throw new WorkflowCallbackUrlUnrecoverableException(runId, waitId); + + RecordReveal(runId, waitId); + + return new WorkflowCallbackUrl { WaitId = waitId, Path = RoutePrefix + token }; + } + + /// Tenancy and state in one read: a wait on another team's run, of another kind, or no longer pending is the same 404. + private async Task LoadPendingCallbackAsync(Guid runId, Guid waitId, Guid teamId, CancellationToken cancellationToken) => + await _db.WorkflowRunWait.AsNoTracking() + .Where(w => w.Id == waitId && w.RunId == runId && w.WaitKind == WorkflowWaitKinds.Callback && w.Status == WorkflowWaitStatuses.Pending) + .Where(w => _db.WorkflowRun.Any(r => r.Id == runId && r.TeamId == teamId)) + .Select(w => new StoredCallbackToken(w.Token, w.TokenCiphertext)) + .SingleOrDefaultAsync(cancellationToken).ConfigureAwait(false) + ?? throw new KeyNotFoundException($"No pending callback wait {waitId} on run {runId} in team {teamId}."); + + /// + /// The only durable account of a reveal. Warning rather than Information because the question it answers is asked after + /// an incident — "who could have resumed that run" — and an Information line is the first thing a retention policy drops. + /// + private void RecordReveal(Guid runId, Guid waitId) => + _logger.LogWarning( + "Callback URL for wait {WaitId} on run {RunId} was revealed to user {UserId} ({UserName}). Anyone holding it can resume the run without signing in.", + waitId, runId, _currentUser.Id, _currentUser.Name); + + private sealed record StoredCallbackToken(string Token, string? TokenCiphertext); +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackUrlUnrecoverableException.cs b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackUrlUnrecoverableException.cs new file mode 100644 index 000000000..b6b48c1c2 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Callbacks/WorkflowCallbackUrlUnrecoverableException.cs @@ -0,0 +1,19 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Core.Services.Workflows.Callbacks; + +/// +/// The pending callback wait exists, but its row holds neither a ciphertext nor the token itself — only its digest, which +/// cannot be turned back into a URL. Every row this version or an earlier one writes keeps the token, so this needs a row +/// whose token column a later release blanked before sealing it, read after rolling back onto this version. The URL +/// already handed out still resumes the run. 409, because it is the row's state that refuses, not the caller. +/// +public sealed class WorkflowCallbackUrlUnrecoverableException : Exception, IFailure +{ + public WorkflowCallbackUrlUnrecoverableException(Guid runId, Guid waitId) + : base($"The callback URL of wait {waitId} on run {runId} is no longer stored in a form that can be shown. The URL already handed out still resumes the run; to resume it by hand, re-issue the wait with a body.") { } + + public FailureKind Kind => FailureKind.Conflict; + + public string Code => FailureCodes.CallbackUrlUnrecoverable; +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Display/WorkflowRunPendingWaitObservationReader.cs b/backend/src/CodeSpace.Core/Services/Workflows/Display/WorkflowRunPendingWaitObservationReader.cs index 22045eee0..1dc6bfa7e 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Display/WorkflowRunPendingWaitObservationReader.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Display/WorkflowRunPendingWaitObservationReader.cs @@ -70,9 +70,10 @@ LIMIT 1 } } + /// A Callback's token alone resumes the run anonymously, so it is never projected; only the RunsControl-gated reveal hands it out. private static WorkflowRunPendingWait ReadWait(DbDataReader reader) => new() { - Id = reader.GetGuid(1), NodeId = reader.GetString(2), Kind = reader.GetString(3), Token = reader.GetString(4), + Id = reader.GetGuid(1), NodeId = reader.GetString(2), Kind = reader.GetString(3), Token = WorkflowWaitKinds.ProjectedToken(reader.GetString(3), reader.GetString(4)), WakeAt = reader.IsDBNull(5) ? null : reader.GetFieldValue(5), PromptState = Enum.TryParse(reader.GetString(6), out var state) ? state : WorkflowRunPendingWaitPromptState.Invalid, PromptPrefix = reader.IsDBNull(7) ? null : reader.GetString(7), diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs b/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs index c83976bea..c126ca945 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs @@ -3062,6 +3062,7 @@ private async Task SuspendNodeAsync(NodeExecution exec, NodeResult result, Cance WorkflowWaitKinds.AgentRun => (await StageAgentRunAsync(exec, token, iterationKey, stagingCt).ConfigureAwait(false)).ToString(), _ => token.CorrelationToken ?? Guid.NewGuid().ToString("N"), }; + var bearerToken = MintBearerToken(waitKind); var waitId = Guid.NewGuid(); var wait = new WorkflowRunWait @@ -3071,7 +3072,9 @@ private async Task SuspendNodeAsync(NodeExecution exec, NodeResult result, Cance NodeId = node.Id, IterationKey = iterationKey, WaitKind = waitKind, - Token = correlationToken, + Token = bearerToken?.Token ?? correlationToken, + TokenDigest = bearerToken?.Digest, + TokenCiphertext = bearerToken?.Ciphertext, WakeAt = wakeAt, Status = WorkflowWaitStatuses.Pending, // Stash the node's suspend payload (e.g. an approval prompt) so the run-detail UI can @@ -3101,6 +3104,15 @@ private async Task SuspendNodeAsync(NodeExecution exec, NodeResult result, Cance _backgroundJobClient.Schedule(s => s.ResumeByDeadlineAsync(waitId, timeoutPayload.GetRawText(), CancellationToken.None), token.DeadlineAt.Value); } + /// + /// A wait whose token alone resolves it (a Callback, resumed by an anonymous POST) also keeps the token's digest, which + /// a resume looks it up by, and its ciphertext under the environment-held key, which only the RunsControl-gated reveal + /// opens. The token itself still goes in the token column, where a previous-version pod looks it up, until a later + /// release blanks that column (see migration 0248). Null for every other kind, whose token is stored as minted. + /// + private SealedCallbackToken? MintBearerToken(string waitKind) => + WorkflowWaitKinds.HasBearerToken(waitKind) ? _lifetimeScope.Resolve().Mint() : null; + /// /// The wait row is a persistence boundary like every ledger row: a Viewer reads it back as the run's pending wait, /// and the payload a node suspends with is built from RESOLVED inputs and config — an agent's task, a child's diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowResumeService.cs b/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowResumeService.cs index 009558bba..a5e378b3b 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowResumeService.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowResumeService.cs @@ -2,6 +2,7 @@ using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Middlewares.Transactional; using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.Workflows.Callbacks; using CodeSpace.Core.Services.Workflows.Dispatch; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Enums; @@ -226,8 +227,14 @@ private async Task ResolveWaitThenDispatchAsync(Guid runId, Guid waitId, s public async Task ResumeByCallbackTokenAsync(string token, string bodyJson, CancellationToken cancellationToken) { + // By digest, the only form left once a later release blanks the token column; or by the token column itself, for a + // row minted before migration 0248 or parked by a previous-version pod, which has no digest. A digest presented as a + // token matches neither, so it never resumes its own wait. + var digest = WorkflowCallbackTokenSealer.Digest(token); + var plaintext = WorkflowCallbackTokenSealer.PlaintextFormOf(token); + var wait = await _db.WorkflowRunWait.AsNoTracking() - .Where(w => w.Token == token && w.Status == WorkflowWaitStatuses.Pending && w.WaitKind == WorkflowWaitKinds.Callback) + .Where(w => (w.TokenDigest == digest || w.Token == plaintext) && w.Status == WorkflowWaitStatuses.Pending && w.WaitKind == WorkflowWaitKinds.Callback) .FirstOrDefaultAsync(cancellationToken) .ConfigureAwait(false); diff --git a/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs b/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs index d3e10b7ef..c47fc341f 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs @@ -2003,7 +2003,8 @@ private Expression> HasPendingDecisionPredicate() => r = Id = pending.Id, NodeId = pending.NodeId, Kind = pending.WaitKind, - Token = pending.Token, + // A Callback's token alone resumes the run anonymously; only the RunsControl-gated reveal hands it out. + Token = WorkflowWaitKinds.ProjectedToken(pending.WaitKind, pending.Token), WakeAt = pending.WakeAt, Payload = SafeParseJson(pending.PayloadJson), }, diff --git a/backend/src/CodeSpace.Core/Settings/Application/PublicBaseUrlSetting.cs b/backend/src/CodeSpace.Core/Settings/Application/PublicBaseUrlSetting.cs index d2d21506d..b5d3153c0 100644 --- a/backend/src/CodeSpace.Core/Settings/Application/PublicBaseUrlSetting.cs +++ b/backend/src/CodeSpace.Core/Settings/Application/PublicBaseUrlSetting.cs @@ -26,6 +26,12 @@ public class PublicBaseUrlSetting : IConfigurationSetting public const string DevelopmentDefault = "http://localhost:5180"; + /// The SPA route an invitation link opens, ahead of its token. + public const string InvitePath = "/invite/"; + + /// The SPA route a password-reset link opens, ahead of its token. + public const string PasswordResetPath = "/reset-password/"; + public PublicBaseUrlSetting(IConfiguration configuration) { Value = Resolve(configuration); @@ -34,10 +40,10 @@ public PublicBaseUrlSetting(IConfiguration configuration) public string Value { get; set; } /// The link an invitee follows. The path is ours; only the origin is configured. - public string InviteUrl(string token) => $"{Value}/invite/{Uri.EscapeDataString(token)}"; + public string InviteUrl(string token) => $"{Value}{InvitePath}{Uri.EscapeDataString(token)}"; /// The link someone locked out follows. - public string PasswordResetUrl(string token) => $"{Value}/reset-password/{Uri.EscapeDataString(token)}"; + public string PasswordResetUrl(string token) => $"{Value}{PasswordResetPath}{Uri.EscapeDataString(token)}"; private static string Resolve(IConfiguration configuration) { diff --git a/backend/src/CodeSpace.Messages/Commands/Workflows/RevealWorkflowCallbackUrlCommand.cs b/backend/src/CodeSpace.Messages/Commands/Workflows/RevealWorkflowCallbackUrlCommand.cs new file mode 100644 index 000000000..a94c46dc6 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Commands/Workflows/RevealWorkflowCallbackUrlCommand.cs @@ -0,0 +1,29 @@ +using CodeSpace.Messages.Authorization; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Dtos.Workflows; +using CodeSpace.Messages.Mediation; + +namespace CodeSpace.Messages.Commands.Workflows; + +/// +/// Hand back the URL of a run's pending Callback wait. +/// +/// A command rather than a query, though it reads: the URL's token is a bearer credential — the callback route is +/// anonymous, so whoever holds it resumes the run with a body of their choosing — so handing it out is an ACTION, and it +/// leaves a log line naming who took it. Queries are gated by membership alone, which would hand it to Viewers. +/// +/// because holding the URL is the power to resume the run, which is what +/// that permission gates (a Member can already force-resolve the same wait with a body of their own through +/// ). +/// +/// Tenancy: the wait's run must belong to the caller's current team, and the wait must still be a pending +/// Callback — anything else is 404. +/// +public sealed record RevealWorkflowCallbackUrlCommand : ICommand, IRequireTeamPermission +{ + public string RequiredPermission => TeamPermissions.RunsControl; + + public required Guid RunId { get; init; } + + public required Guid WaitId { get; init; } +} diff --git a/backend/src/CodeSpace.Messages/Constants/WorkflowWaitConstants.cs b/backend/src/CodeSpace.Messages/Constants/WorkflowWaitConstants.cs index 764566289..5ffc67403 100644 --- a/backend/src/CodeSpace.Messages/Constants/WorkflowWaitConstants.cs +++ b/backend/src/CodeSpace.Messages/Constants/WorkflowWaitConstants.cs @@ -94,6 +94,28 @@ public static class WorkflowWaitKinds /// Pinned by a unit test so widening the set is a conscious, reviewed decision (Rule 8). /// public static bool IsOperatorReissuable(string waitKind) => waitKind is Timer or Callback; + + /// + /// The wait kinds whose token is a BEARER credential: presenting it alone, with no session, resolves the wait. Only + /// — its route is anonymous, so whoever holds the token resumes the run with a body of their + /// choosing. Such a token is never projected to a reader (only the RunsControl-gated reveal hands it out), and its row + /// also keeps a digest, which a resume looks it up by, and a ciphertext, which the reveal opens. Every other kind's + /// token is a run id (, ) or is acted on through an authenticated, + /// team-scoped verb that needs runs.decide. Pinned per kind by a unit test so adding a kind is a reviewed decision. + /// + public static bool HasBearerToken(string waitKind) => waitKind is Callback; + + /// + /// What a run read shows in place of a bearer token (). Not null: every SPA build since the + /// pending-wait decoder shipped refuses a wait whose token is not a 1..128-character string, and a previous-version SPA + /// served during the deploy would show "Pending action unavailable" for every Callback wait. It says what to do on that + /// SPA, whose run page builds the callback URL from it — a URL that resumes nothing: no version mints this shape, and + /// this version's callback route admits only 32 characters. Pinned by a unit test, and decoded by the frontend's. + /// + public const string WithheldBearerToken = "withheld-reload-the-page-to-reveal"; + + /// The token a run read projects for a wait of : for a bearer token, the stored token otherwise. + public static string ProjectedToken(string waitKind, string token) => HasBearerToken(waitKind) ? WithheldBearerToken : token; } /// Lifecycle of a workflow_run_wait row. CHECK-constrained at the DB layer. A wait leaves one of two ways, and only carries an answer. diff --git a/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowCallbackUrl.cs b/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowCallbackUrl.cs new file mode 100644 index 000000000..b5001d326 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowCallbackUrl.cs @@ -0,0 +1,13 @@ +namespace CodeSpace.Messages.Dtos.Workflows; + +/// +/// A pending callback wait's URL, handed out by RevealWorkflowCallbackUrlCommand. is the path +/// on this API (/api/workflows/callbacks/{token}); the caller puts the origin it reaches the API on in front of it, +/// as the run page always did. Anyone holding it can resume the run without signing in. +/// +public sealed record WorkflowCallbackUrl +{ + public required Guid WaitId { get; init; } + + public required string Path { get; init; } +} diff --git a/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunDetail.cs b/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunDetail.cs index 6c3ddc254..90860fe80 100644 --- a/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunDetail.cs +++ b/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunDetail.cs @@ -80,8 +80,9 @@ public sealed record WorkflowRunWaitInfo public required string Kind { get; init; } /// - /// The correlation token. For a Callback wait it's the bearer secret the UI builds the - /// callback URL from (the run owner shares it with the external system). + /// The correlation token — for a Subworkflow wait the child run's id, for an AgentRun wait the agent run's id. For a + /// Callback wait it is WorkflowWaitKinds.WithheldBearerToken: the token alone resumes the run without a session, + /// so it is handed out only by RevealWorkflowCallbackUrlCommand, which needs runs.control. /// public required string Token { get; init; } diff --git a/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunPendingWaitObservation.cs b/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunPendingWaitObservation.cs index 6445bcea3..c35f706a1 100644 --- a/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunPendingWaitObservation.cs +++ b/backend/src/CodeSpace.Messages/Dtos/Workflows/WorkflowRunPendingWaitObservation.cs @@ -13,6 +13,7 @@ public sealed record WorkflowRunPendingWait public required Guid Id { get; init; } public required string NodeId { get; init; } public required string Kind { get; init; } + /// The correlation token (a Subworkflow wait's child run id, …). WorkflowWaitKinds.WithheldBearerToken for a Callback wait, whose token is a bearer credential handed out only by RevealWorkflowCallbackUrlCommand. public required string Token { get; init; } public DateTimeOffset? WakeAt { get; init; } public required WorkflowRunPendingWaitPromptState PromptState { get; init; } diff --git a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs index 54de365e8..eda5ff993 100644 --- a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs +++ b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs @@ -87,6 +87,9 @@ public static class FailureCodes /// A settled cell's outputs were redacted for persistence and their encrypted recovery payload is not readable, so the originals exist nowhere. Remedy: re-run the workflow from that node — a resume can only ever offer the redaction placeholder. public const string WorkflowOutputsUnrecoverable = "workflow_outputs_unrecoverable"; + /// A callback wait's row keeps only its token's digest — no ciphertext, no token — so its URL cannot be shown again. The URL already handed out still resumes the run. Remedy: use that URL, or re-issue the wait with a body to resume the run by hand. + public const string CallbackUrlUnrecoverable = "callback_url_unrecoverable"; + // ── Invitations ──────────────────────────────────────────────────────────────── public const string InvitationNotUsable = "invitation_not_usable"; public const string InvitationEmailMismatch = "invitation_email_mismatch"; diff --git a/backend/src/CodeSpace.Messages/Workflows/SealedCallbackToken.cs b/backend/src/CodeSpace.Messages/Workflows/SealedCallbackToken.cs new file mode 100644 index 000000000..1f550847d --- /dev/null +++ b/backend/src/CodeSpace.Messages/Workflows/SealedCallbackToken.cs @@ -0,0 +1,10 @@ +namespace CodeSpace.Messages.Workflows; + +/// +/// A freshly minted callback-wait token in the three forms its wait row stores. is the bearer secret the +/// callback URL carries, kept in the token column for one release so a pod on the previous build — which compares that +/// column with the presented token — still resumes the URL; (its SHA-256) is what a resume looks the +/// wait up by; is the token AES-GCM encrypted (base64) under the deployment's environment-held +/// variable master key, for the RunsControl-gated reveal — nothing the database holds opens it. +/// +public sealed record SealedCallbackToken(string Token, string Digest, string Ciphertext); diff --git a/backend/tests/CodeSpace.E2ETests/Auth/BearerUrlTokenLogE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Auth/BearerUrlTokenLogE2ETests.cs new file mode 100644 index 000000000..c6a911914 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Auth/BearerUrlTokenLogE2ETests.cs @@ -0,0 +1,346 @@ +using System.Collections.Concurrent; +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using System.Text; +using System.Text.Json; +using CodeSpace.Api.Logging; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Users; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Serilog; +using Serilog.Core; +using Serilog.Events; +using Shouldly; + +namespace CodeSpace.E2ETests.Auth; + +/// +/// Invitation, password-reset and workflow-callback tokens are bearer credentials that travel in URL PATHS. Every request +/// used to write its raw path to the console and Seq — in Serilog's completion line and as RequestPath on every +/// event logged while it ran — so anyone who could read the logs could join a team, take over an account or resume a run. +/// This drives each of those routes through the real pipeline, on success and on failure, with the process logger built +/// the way Program.BuildLogger builds it (the shipped Serilog settings and Enrich.FromLogContext), and +/// reads back every event the sink received. It also sends each token where no bearer route answers — a GET or HEAD on +/// the callback URL, a wrong method, the SPA's links on the API origin — and as the first request of a fresh +/// non-Development pod, whose https-port warning is logged before routing. +/// +/// Tier: High fidelity. The real app host and its middleware order, real Postgres, the real engine drained for the +/// callback run, the real invitation, reset and callback services. The sink is the only addition: it replaces console +/// and Seq, which receive exactly these events. The suite runs serially (AssemblyParallelization), so swapping the +/// process-wide Log.Logger for the length of the test cannot capture another test's events. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class BearerUrlTokenLogE2ETests +{ + [Fact] + public async Task Invitation_reset_and_callback_tokens_in_url_paths_reach_no_log_event_on_success_or_failure() + { + var sink = new CapturedEvents(); + var previous = Log.Logger; + Log.Logger = ProductionShapedLogger(sink); + var factory = new TaskLaunchApiFactory(); + await ((IAsyncLifetime)factory).InitializeAsync(); + + try + { + var world = await SeedWorldAsync(factory); + var anonymous = factory.CreateClient(); + + var invitation = TokenAfterLastSlash(await ReadStringAsync(await SendAsync(factory, HttpMethod.Post, "/api/teams/invitations", world, new { email = $"invitee-{world.Suffix}@test.local", role = "Member" }), "inviteUrl")); + (await anonymous.GetAsync($"/api/invitations/{invitation}")).StatusCode.ShouldBe(HttpStatusCode.OK, "preview"); + (await anonymous.PostAsJsonAsync($"/api/invitations/{invitation}/accept", new { name = "Too Short", password = "x" })).StatusCode.ShouldBe(HttpStatusCode.BadRequest, "a refused accept leaves the token valid — the case where a logged token is still usable"); + (await anonymous.PostAsJsonAsync($"/api/invitations/{invitation}/accept", new { name = $"Joined {world.Suffix}", password = "correct-horse-battery" })).StatusCode.ShouldBe(HttpStatusCode.OK, "accept"); + + var reset = TokenAfterLastSlash((await IssueResetLinkAsync(factory, world.MemberId)).ResetUrl); + (await anonymous.PostAsJsonAsync($"/api/auth/reset-password/{reset}", new { newPassword = "short" })).StatusCode.ShouldBe(HttpStatusCode.BadRequest, "a refused reset leaves the token valid"); + (await anonymous.PostAsJsonAsync($"/api/auth/reset-password/{reset}", new { newPassword = "recovered-passphrase-x" })).StatusCode.ShouldBe(HttpStatusCode.NoContent, "reset"); + + var callbackPath = await ParkCallbackAndRevealAsync(factory, world); + var callback = TokenAfterLastSlash(callbackPath); + var guessed = Guid.NewGuid().ToString("N"); + (await anonymous.PostAsync($"/api/workflows/callbacks/{guessed}", Json("{}"))).StatusCode.ShouldBe(HttpStatusCode.NotFound, "a guessed callback token"); + (await anonymous.PostAsync(callbackPath, Json("""{"status":"done"}"""))).StatusCode.ShouldBe(HttpStatusCode.OK, "the callback"); + + // The capture is live and saw these requests — so the absence below is a measurement, not an empty sink. + sink.CompletionPaths().ShouldContain("/api/invitations/{token}", customMessage: "the preview's completion line logs the route template. If the sink holds no completion lines at all, Log.Logger was not the host's logger — check TaskLaunchApiFactory still builds through Program.CreateHostBuilder's UseSerilog()"); + sink.CompletionPaths().ShouldContain("/api/invitations/{token}/accept"); + sink.CompletionPaths().ShouldContain("/api/auth/reset-password/{token}"); + sink.CompletionPaths().ShouldContain("/api/workflows/callbacks/{token:length(32)}"); + sink.Events.ShouldContain(e => e.Properties.ContainsKey("RequestName") && e.Properties["RequestPath"].ToString() == "\"/api/invitations/{token}/accept\"", "a mediator line inside the accept carries the template as its hosting-scope RequestPath"); + + foreach (var (name, token) in new[] { ("invitation", invitation), ("reset", reset), ("callback", callback), ("guessed callback", guessed) }) + sink.Rendered.ShouldNotContain(token, customMessage: $"the {name} token from a URL path reached a log event:\n{sink.LinesContaining(token)}"); + } + finally + { + Log.Logger = previous; + await ((IAsyncLifetime)factory).DisposeAsync(); + } + } + + [Fact] + public async Task Valid_tokens_sent_with_a_method_or_on_a_path_no_bearer_route_answers_reach_no_log_event() + { + // The fallback answers these, not a bearer route: a link unfurler or a person pasting the callback URL into a browser + // sends GET, an uptime check HEAD; a wrong method on an invitation or reset route; the SPA's invite or reset link + // built against the API origin. Each token is still valid afterwards — what a log reader would hold is live. + var sink = new CapturedEvents(); + var previous = Log.Logger; + Log.Logger = ProductionShapedLogger(sink); + var factory = new TaskLaunchApiFactory(); + await ((IAsyncLifetime)factory).InitializeAsync(); + + try + { + var world = await SeedWorldAsync(factory); + var anonymous = factory.CreateClient(); + + var invitation = TokenAfterLastSlash(await ReadStringAsync(await SendAsync(factory, HttpMethod.Post, "/api/teams/invitations", world, new { email = $"invitee-{world.Suffix}@test.local", role = "Member" }), "inviteUrl")); + var reset = TokenAfterLastSlash((await IssueResetLinkAsync(factory, world.MemberId)).ResetUrl); + var callbackPath = await ParkCallbackAndRevealAsync(factory, world); + var callback = TokenAfterLastSlash(callbackPath); + + await anonymous.GetAsync(callbackPath); + await anonymous.SendAsync(new HttpRequestMessage(HttpMethod.Head, callbackPath)); + await anonymous.GetAsync($"/api/invitations/{invitation}/accept"); + await anonymous.GetAsync($"/api/auth/reset-password/{reset}"); + await anonymous.GetAsync($"/invite/{invitation}"); + await anonymous.GetAsync($"/reset-password/{reset}"); + + (await anonymous.GetAsync($"/api/invitations/{invitation}")).StatusCode.ShouldBe(HttpStatusCode.OK, "the invitation token stayed valid"); + (await anonymous.PostAsync(callbackPath, Json("""{"status":"done"}"""))).StatusCode.ShouldBe(HttpStatusCode.OK, "the callback token stayed valid"); + + var paths = sink.CompletionPaths(); + paths.Count(p => p == "/api/workflows/callbacks/{token}").ShouldBe(2, $"the GET and the HEAD on the callback URL each logged a completion line, masked. Logged: {string.Join(", ", paths)}"); + paths.ShouldContain("/api/invitations/{token}/accept"); + paths.ShouldContain("/api/auth/reset-password/{token}"); + paths.ShouldContain("/invite/{token}"); + paths.ShouldContain("/reset-password/{token}"); + + foreach (var (name, token) in new[] { ("invitation", invitation), ("reset", reset), ("callback", callback) }) + sink.Rendered.ShouldNotContain(token, customMessage: $"the {name} token from a URL path reached a log event:\n{sink.LinesContaining(token)}"); + } + finally + { + Log.Logger = previous; + await ((IAsyncLifetime)factory).DisposeAsync(); + } + } + + [Fact] + public async Task The_first_request_of_a_fresh_non_development_api_logs_no_token_before_routing() + { + // UseHttpsRedirection runs ahead of routing and, in a non-Development host with no https port (the shipped compose + // API: Production, ASPNETCORE_URLS=http://+:8080), warns once per process — on whichever request comes first. A + // sibling host on the same database stands in for a pod that just started; its first request is a callback. + var sink = new CapturedEvents(); + var previous = Log.Logger; + Log.Logger = ProductionShapedLogger(sink); + var factory = new TaskLaunchApiFactory(); + await ((IAsyncLifetime)factory).InitializeAsync(); + NonDevelopmentApiFactory? freshPod = null; + + try + { + var world = await SeedWorldAsync(factory); + var callbackPath = await ParkCallbackAndRevealAsync(factory, world); + var callback = TokenAfterLastSlash(callbackPath); + + freshPod = new NonDevelopmentApiFactory(factory.ConnectionString); + var client = freshPod.CreateClient(new Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); + (await client.PostAsync(callbackPath, Json("""{"status":"done"}"""))).StatusCode.ShouldBe(HttpStatusCode.OK, "the fresh pod's first request is the callback, and it resumes the run"); + + sink.Events.ShouldContain(e => SourceOf(e) == "Microsoft.AspNetCore.HttpsPolicy.HttpsRedirectionMiddleware" && e.Properties["RequestPath"].ToString() == "\"/api/workflows/callbacks/{token}\"", $"the https-port warning fired on the callback and carried the masked path. If no such event exists at all, the host found an https port and this test measures nothing — check NonDevelopmentApiFactory still runs outside Development. Events:\n{sink.Rendered}"); + sink.Rendered.ShouldNotContain(callback, customMessage: $"the callback token reached a log event:\n{sink.LinesContaining(callback)}"); + } + finally + { + Log.Logger = previous; + if (freshPod != null) await freshPod.DisposeAsync(); + await ((IAsyncLifetime)factory).DisposeAsync(); + } + } + + [Fact] + public async Task Every_route_that_binds_a_credential_looking_parameter_is_redacted_from_the_log() + { + // The redaction is keyed on the route parameter's NAME. A new route that names a credential anything else would be + // logged raw, so every real endpoint whose parameter reads like one must be one the redaction recognises. + var factory = new TaskLaunchApiFactory(); + await ((IAsyncLifetime)factory).InitializeAsync(); + + try + { + var routes = factory.Services.GetRequiredService().Endpoints.OfType().ToList(); + var credentialShaped = routes.Where(r => r.RoutePattern.Parameters.Any(p => p.Name.Contains("token", StringComparison.OrdinalIgnoreCase) || p.Name.Contains("secret", StringComparison.OrdinalIgnoreCase))).ToList(); + var templates = credentialShaped.Select(r => r.RoutePattern.RawText).ToList(); + + templates.ShouldContain("api/invitations/{token}"); + templates.ShouldContain("api/invitations/{token}/accept"); + templates.ShouldContain("api/auth/reset-password/{token}"); + templates.ShouldContain("api/workflows/callbacks/{token:length(32)}"); + + foreach (var route in credentialShaped) + route.RoutePattern.Parameters.ShouldContain(p => BearerRouteLogging.BearerParameters.Contains(p.Name), $"{route.RoutePattern.RawText} binds a credential-looking parameter the log redaction does not key on; name it 'token' or add its name to BearerRouteLogging.BearerParameters"); + } + finally + { + await ((IAsyncLifetime)factory).DisposeAsync(); + } + } + + // ─── World ────────────────────────────────────────────────────────────────── + + private sealed record World(Guid TeamId, Guid OwnerId, Guid MemberId, string Suffix); + + /// The process logger as Program.BuildLogger makes it: shipped levels, Enrich.FromLogContext, and a sink in place of console + Seq. + private static Serilog.ILogger ProductionShapedLogger(ILogEventSink sink) + { + var shipped = new ConfigurationBuilder().AddJsonFile(Path.Combine(FindBackendRoot(), "src", "CodeSpace.Api", "appsettings.json")).Build(); + + return new LoggerConfiguration().ReadFrom.Configuration(shipped).Enrich.FromLogContext().WriteTo.Sink(sink).CreateLogger(); + } + + private static async Task SeedWorldAsync(TaskLaunchApiFactory factory) + { + using var scope = factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var teamId = Guid.NewGuid(); + var suffix = teamId.ToString("N")[..8]; + db.Team.Add(new Team { Id = teamId, Slug = $"logs-{suffix}", Name = "Logs", Kind = TeamKind.Workspace, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + + Guid Member(TeamRole role) + { + var userId = Guid.NewGuid(); + db.User.Add(new User { Id = userId, SecurityStamp = TestToken.SeedStamp, Email = $"logs-{role}-{suffix}@test.local", Name = $"logs {role} {suffix}", CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = teamId, UserId = userId, Role = role, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + return userId; + } + + var world = new World(teamId, Member(TeamRole.Owner), Member(TeamRole.Member), suffix); + await db.SaveChangesAsync(); + + return world; + } + + /// Issued through the service, as SessionRevocationE2ETests does: the admin endpoint needs a deployment admin, and the token's route is what is under test. + private static async Task IssueResetLinkAsync(TaskLaunchApiFactory factory, Guid userId) + { + using var scope = factory.Services.CreateScope(); + return await scope.ServiceProvider.GetRequiredService().IssueResetAsync(userId, CancellationToken.None); + } + + /// Owner authors and runs trigger → flow.wait_callback → terminal; the engine parks it; the Owner reveals the URL. + private static async Task ParkCallbackAndRevealAsync(TaskLaunchApiFactory factory, World world) + { + var workflow = new + { + name = $"logs-{Guid.NewGuid():N}"[..20], + definition = new + { + schemaVersion = 1, + nodes = new[] + { + new { id = "start", typeKey = "trigger.manual", config = new { }, inputs = new { } }, + new { id = "cb", typeKey = "flow.wait_callback", config = new { }, inputs = new { } }, + new { id = "end", typeKey = "builtin.terminal", config = new { }, inputs = new { } }, + }, + edges = new[] { new { from = "start", to = "cb" }, new { from = "cb", to = "end" } }, + }, + activations = Array.Empty(), + enabled = true, + }; + + var workflowId = Guid.Parse(await ReadStringAsync(await SendAsync(factory, HttpMethod.Post, "/api/workflows", world, workflow), "id")); + var runId = Guid.Parse(await ReadStringAsync(await SendAsync(factory, HttpMethod.Post, $"/api/workflows/{workflowId}/run", world, new { }), "runId")); + await factory.JobClient.DrainAsync(); + + var pending = await SendAsync(factory, HttpMethod.Get, $"/api/workflows/runs/{runId}/pending-wait", world); + var waitId = JsonDocument.Parse(await pending.Content.ReadAsStringAsync()).RootElement.GetProperty("wait").GetProperty("id").GetGuid(); + + return await ReadStringAsync(await SendAsync(factory, HttpMethod.Post, $"/api/workflows/runs/{runId}/waits/{waitId}/callback-url", world), "path"); + } + + private static async Task SendAsync(TaskLaunchApiFactory factory, HttpMethod method, string url, World world, object? body = null) + { + using var request = new HttpRequestMessage(method, url); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", TestToken.Mint(world.OwnerId, TestToken.SeedStamp)); + request.Headers.Add("X-Team-Id", world.TeamId.ToString()); + if (body != null) request.Content = JsonContent.Create(body); + + return await factory.CreateClient().SendAsync(request); + } + + private static async Task ReadStringAsync(HttpResponseMessage response, string property) + { + var text = await response.Content.ReadAsStringAsync(); + response.IsSuccessStatusCode.ShouldBeTrue($"{response.RequestMessage?.Method} {response.RequestMessage?.RequestUri?.AbsolutePath} answered {(int)response.StatusCode}: {text}"); + + var value = JsonDocument.Parse(text).RootElement.GetProperty(property); + return value.ValueKind == JsonValueKind.String ? value.GetString()! : value.GetRawText(); + } + + private static string TokenAfterLastSlash(string url) => url[(url.LastIndexOf('/') + 1)..]; + + private static StringContent Json(string body) => new(body, Encoding.UTF8, "application/json"); + + private static string? SourceOf(LogEvent logEvent) => logEvent.Properties.TryGetValue("SourceContext", out var source) ? ((ScalarValue)source).Value?.ToString() : null; + + /// The app as a deployed API pod runs it — Production, so UseHttpsRedirection is on — over another fixture's database; never initialised as a fixture of its own. + private sealed class NonDevelopmentApiFactory : TaskLaunchApiFactory + { + private readonly string _connectionString; + + public NonDevelopmentApiFactory(string connectionString) { _connectionString = connectionString; } + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + base.ConfigureWebHost(builder); + builder.UseEnvironment("Production"); + builder.ConfigureAppConfiguration((_, configuration) => configuration.AddInMemoryCollection(new Dictionary + { + ["CodeSpaceStore:ConnectionString"] = _connectionString, + [CodeSpace.Core.Settings.Application.PublicBaseUrlSetting.ConfigurationKey] = "http://spa.test", + [CodeSpace.Core.Settings.Webhooks.WebhookBaseUrlSetting.ConfigurationKey] = "http://api.test", + })); + } + } + + private static string FindBackendRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "CodeSpace.sln"))) + directory = directory.Parent; + + return directory?.FullName ?? throw new DirectoryNotFoundException("Could not locate the CodeSpace backend root."); + } + + /// Every event the host's logger let through, rendered with every property — what a console template or a Seq query could show. + private sealed class CapturedEvents : ILogEventSink + { + public ConcurrentQueue Events { get; } = new(); + + public string Rendered => string.Join("\n", Lines()); + + public IReadOnlyList CompletionPaths() => + Events.Where(e => e.Properties.ContainsKey("StatusCode") && e.Properties.ContainsKey("Elapsed") && e.Properties.ContainsKey("RequestPath")) + .Select(e => ((ScalarValue)e.Properties["RequestPath"]).Value?.ToString() ?? "") + .ToList(); + + public string LinesContaining(string value) => string.Join("\n", Lines().Where(l => l.Contains(value, StringComparison.Ordinal))); + + public void Emit(LogEvent logEvent) => Events.Enqueue(logEvent); + + private IEnumerable Lines() => + Events.Select(e => $"[{e.Level}] {e.RenderMessage()} {e.Exception} :: {string.Join(" | ", e.Properties.Select(p => $"{p.Key}={p.Value}"))}"); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs index 5e00f51c0..a7e18f205 100644 --- a/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs +++ b/backend/tests/CodeSpace.E2ETests/Webhooks/WebhookOutsiderHardeningE2ETests.cs @@ -8,6 +8,7 @@ using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Workflows.Callbacks; using CodeSpace.Core.Services.Workflows.Engine; using CodeSpace.E2ETests.Infrastructure; using CodeSpace.IntegrationTests.Webhooks; @@ -491,10 +492,13 @@ private async Task LoadOnlyRunIdAsync(Guid workflowId) return await scope.ServiceProvider.GetRequiredService().WorkflowRun.AsNoTracking().Where(r => r.WorkflowId == workflowId).Select(r => r.Id).SingleAsync(); } + /// The token the engine minted, read back the way the reveal does — the row itself holds only its digest and ciphertext. 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(); + var stored = await scope.ServiceProvider.GetRequiredService().WorkflowRunWait.AsNoTracking().Where(w => w.RunId == runId && w.WaitKind == WorkflowWaitKinds.Callback).Select(w => new { w.Token, w.TokenCiphertext }).SingleAsync(); + + return scope.ServiceProvider.GetRequiredService().Open(stored.Token, stored.TokenCiphertext)!; } private async Task> LoadRefusalsAsync(RepositorySeed seed) => (await LoadRejectedRowsAsync(seed)).Select(r => r.Error!).ToList(); diff --git a/backend/tests/CodeSpace.E2ETests/Workflows/CallbackUrlRevealE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Workflows/CallbackUrlRevealE2ETests.cs new file mode 100644 index 000000000..fb41b687b --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Workflows/CallbackUrlRevealE2ETests.cs @@ -0,0 +1,225 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using System.Security.Cryptography; +using System.Text; +using System.Text.Json; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Variables; +using CodeSpace.Core.Services.Workflows.Callbacks; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Npgsql; +using Shouldly; + +namespace CodeSpace.E2ETests.Workflows; + +/// +/// A run parked on flow.wait_callback is resumed by an anonymous POST of its token, so the token is the power to +/// resume the run with a body of the caller's choosing. A Viewer used to read it off the run page and do exactly that, +/// bypassing runs.control. This drives the whole path over HTTP: a Viewer reads both run surfaces and finds only the +/// withheld placeholder and is refused the reveal; a Member (who holds runs.control) reveals the URL; an anonymous +/// client resumes the run with it, while a guessed token, the placeholder and the stored digest are 404. +/// +/// Tier: High fidelity. The real app host (JWT, X-Team-Id, the permission behavior, controllers, the anonymous +/// callback route and its rate limiter), real Postgres, the real engine drained behind POST /api/workflows/{id}/run. +/// Nothing is stubbed. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class CallbackUrlRevealE2ETests : IClassFixture +{ + private readonly TaskLaunchApiFactory _factory; + + public CallbackUrlRevealE2ETests(TaskLaunchApiFactory factory) { _factory = factory; } + + [Fact] + public async Task A_viewer_never_sees_the_callback_token_and_the_url_a_member_reveals_resumes_the_run_anonymously() + { + var world = await SeedWorldAsync(); + var runId = await LaunchCallbackRunAsync(world); + await _factory.JobClient.DrainAsync(); + + var viewerDetail = await ReadOkAsync(HttpMethod.Get, $"/api/workflows/runs/{runId}", world.ViewerId, world.TeamId); + var viewerPending = await ReadOkAsync(HttpMethod.Get, $"/api/workflows/runs/{runId}/pending-wait", world.ViewerId, world.TeamId); + var detailWait = JsonDocument.Parse(viewerDetail).RootElement.GetProperty("pendingWait"); + var pendingWait = JsonDocument.Parse(viewerPending).RootElement.GetProperty("wait"); + var waitId = pendingWait.GetProperty("id").GetGuid(); + + detailWait.GetProperty("kind").GetString().ShouldBe(WorkflowWaitKinds.Callback, "precondition: the run is parked on its callback"); + detailWait.GetProperty("token").GetString().ShouldBe(WorkflowWaitKinds.WithheldBearerToken, "GET /runs/{id} must not hand a Viewer the token that resumes the run — a placeholder every SPA build decodes stands in for it"); + pendingWait.GetProperty("token").GetString().ShouldBe(WorkflowWaitKinds.WithheldBearerToken, "GET /runs/{id}/pending-wait must not either"); + + var viewerReveal = await SendAsync(HttpMethod.Post, RevealPath(runId, waitId), world.ViewerId, world.TeamId); + viewerReveal.StatusCode.ShouldBe(HttpStatusCode.Forbidden, "revealing the URL is the power to resume the run, which runs.control gates"); + + var revealed = await SendAsync(HttpMethod.Post, RevealPath(runId, waitId), world.MemberId, world.TeamId); + revealed.StatusCode.ShouldBe(HttpStatusCode.OK, await revealed.Content.ReadAsStringAsync()); + var path = JsonDocument.Parse(await revealed.Content.ReadAsStringAsync()).RootElement.GetProperty("path").GetString()!; + path.ShouldStartWith("/api/workflows/callbacks/"); + var token = path["/api/workflows/callbacks/".Length..]; + + viewerDetail.ShouldNotContain(token, customMessage: "the run detail a Viewer read held the token"); + viewerPending.ShouldNotContain(token, customMessage: "the pending wait a Viewer read held the token"); + var (_, digest, _) = await ReadRowAsADatabaseReaderAsync(waitId); + digest.ShouldBe(WorkflowCallbackTokenSealer.Digest(token)); + + (await PostAnonymouslyAsync($"/api/workflows/callbacks/{Guid.NewGuid():N}", "{}")).StatusCode.ShouldBe(HttpStatusCode.NotFound, "a guessed token resumes nothing"); + (await PostAnonymouslyAsync($"/api/workflows/callbacks/{digest}", "{}")).StatusCode.ShouldBe(HttpStatusCode.NotFound, "the digest a database reader holds is not a key"); + (await PostAnonymouslyAsync($"/api/workflows/callbacks/{WorkflowWaitKinds.WithheldBearerToken}", "{}")).StatusCode.ShouldBe(HttpStatusCode.NotFound, "the URL a previous-version SPA builds from the placeholder resumes nothing"); + + var resumed = await PostAnonymouslyAsync(path, """{"status":"done"}"""); + resumed.StatusCode.ShouldBe(HttpStatusCode.OK, "the revealed URL is the one the external system POSTs to, with no session"); + await _factory.JobClient.DrainAsync(); + + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Success); + (await PostAnonymouslyAsync(path, "{}")).StatusCode.ShouldBe(HttpStatusCode.NotFound, "the wait is resolved; its URL resumes nothing more"); + (await SendAsync(HttpMethod.Post, RevealPath(runId, waitId), world.MemberId, world.TeamId)).StatusCode.ShouldBe(HttpStatusCode.NotFound, "nothing is pending to reveal"); + } + + [Fact] + public async Task Nothing_the_database_holds_opens_the_ciphertext_a_reveal_reads() + { + // A database or backup reader holds the wait row AND the Data Protection key ring, which sits unencrypted in the same + // database (data_protection_keys) under the public application name. The token's ciphertext must not open with + // anything in there — the key that opens it is the one the deployment's environment holds. Once the follow-up + // release blanks the token column, this ciphertext is the only readable copy of the URL. + var world = await SeedWorldAsync(); + var runId = await LaunchCallbackRunAsync(world); + await _factory.JobClient.DrainAsync(); + + var waitId = JsonDocument.Parse(await ReadOkAsync(HttpMethod.Get, $"/api/workflows/runs/{runId}/pending-wait", world.MemberId, world.TeamId)).RootElement.GetProperty("wait").GetProperty("id").GetGuid(); + var path = JsonDocument.Parse(await ReadOkAsync(HttpMethod.Post, RevealPath(runId, waitId), world.MemberId, world.TeamId)).RootElement.GetProperty("path").GetString()!; + var token = path["/api/workflows/callbacks/".Length..]; + var (storedToken, _, ciphertext) = await ReadRowAsADatabaseReaderAsync(waitId); + + // Expand step: the token column keeps the token for one release so a previous-version API pod — which compares it + // verbatim — still resumes the URL during the rollout and after a rollback. The follow-up blanks it; flip this then. + storedToken.ShouldBe(token); + ciphertext.ShouldNotBeNull("precondition: the row carries a ciphertext for the reveal"); + ciphertext.ShouldNotContain(token); + (await TryOpenWithTheDatabaseKeyRingAsync(ciphertext)).ShouldNotBe(token, "the database's own Data Protection key ring must not open the callback token"); + _factory.Services.GetRequiredService().Decrypt(Convert.FromBase64String(ciphertext)).ShouldBe(token, "the reveal opens it with the deployment's environment-held key — so the measurement above is of a real ciphertext"); + } + + // ─── World ────────────────────────────────────────────────────────────────── + + private sealed record World(Guid TeamId, Guid OwnerId, Guid MemberId, Guid ViewerId); + + private static string RevealPath(Guid runId, Guid waitId) => $"/api/workflows/runs/{runId}/waits/{waitId}/callback-url"; + + private async Task SeedWorldAsync() + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var teamId = Guid.NewGuid(); + var suffix = teamId.ToString("N")[..8]; + db.Team.Add(new Team { Id = teamId, Slug = $"cb-{suffix}", Name = "Callback", Kind = TeamKind.Workspace, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + + Guid Member(TeamRole role) + { + var userId = Guid.NewGuid(); + db.User.Add(new User { Id = userId, SecurityStamp = TestToken.SeedStamp, Email = $"cb-{role}-{suffix}@test.local", Name = $"cb {role} {suffix}", CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = teamId, UserId = userId, Role = role, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + return userId; + } + + var world = new World(teamId, Member(TeamRole.Owner), Member(TeamRole.Member), Member(TeamRole.Viewer)); + await db.SaveChangesAsync(); + + return world; + } + + /// The Owner authors trigger → flow.wait_callback → terminal over HTTP and launches it. + private async Task LaunchCallbackRunAsync(World world) + { + var workflow = new + { + name = $"cb-{Guid.NewGuid():N}"[..20], + definition = new + { + schemaVersion = 1, + nodes = new[] + { + new { id = "start", typeKey = "trigger.manual", config = new { }, inputs = new { } }, + new { id = "cb", typeKey = "flow.wait_callback", config = new { }, inputs = new { } }, + new { id = "end", typeKey = "builtin.terminal", config = new { }, inputs = new { } }, + }, + edges = new[] { new { from = "start", to = "cb" }, new { from = "cb", to = "end" } }, + }, + activations = Array.Empty(), + enabled = true, + }; + + var workflowId = JsonDocument.Parse(await ReadOkAsync(HttpMethod.Post, "/api/workflows", world.OwnerId, world.TeamId, workflow)).RootElement.GetProperty("id").GetGuid(); + + return JsonDocument.Parse(await ReadOkAsync(HttpMethod.Post, $"/api/workflows/{workflowId}/run", world.OwnerId, world.TeamId, new { })).RootElement.GetProperty("runId").GetGuid(); + } + + /// What a backup holds: the row, read with nothing but a connection to the database. + private async Task<(string Token, string? Digest, string? Ciphertext)> ReadRowAsADatabaseReaderAsync(Guid waitId) + { + await using var connection = new NpgsqlConnection(_factory.ConnectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand("SELECT token, token_digest, token_ciphertext FROM workflow_run_wait WHERE id = @id", connection); + command.Parameters.AddWithValue("id", waitId); + await using var reader = await command.ExecuteReaderAsync(); + (await reader.ReadAsync()).ShouldBeTrue(); + + return (reader.GetString(0), reader.IsDBNull(1) ? null : reader.GetString(1), reader.IsDBNull(2) ? null : reader.GetString(2)); + } + + /// + /// The key ring the database itself holds, opened the way the app opens it, under the purpose callback tokens were once + /// protected with. Null when it cannot open the value. + /// + private async Task TryOpenWithTheDatabaseKeyRingAsync(string ciphertext) + { + var services = new ServiceCollection(); + services.AddDbContext(options => options.UseNpgsql(_factory.ConnectionString)); + services.AddDataProtection().PersistKeysToDbContext().SetApplicationName(CodeSpaceDataProtection.ApplicationName); + await using var provider = services.BuildServiceProvider(); + + try + { + return provider.GetRequiredService().CreateProtector("CodeSpace.Workflows.CallbackToken.v1").Unprotect(ciphertext); + } + catch (Exception exception) when (exception is CryptographicException or FormatException) + { + return null; + } + } + + private async Task RunStatusAsync(Guid runId) + { + using var scope = _factory.Services.CreateScope(); + return await scope.ServiceProvider.GetRequiredService().WorkflowRun.AsNoTracking().Where(r => r.Id == runId).Select(r => r.Status).SingleAsync(); + } + + private async Task PostAnonymouslyAsync(string path, string body) => + await _factory.CreateClient().PostAsync(path, new StringContent(body, Encoding.UTF8, "application/json")); + + private async Task ReadOkAsync(HttpMethod method, string url, Guid userId, Guid teamId, object? body = null) + { + var response = await SendAsync(method, url, userId, teamId, body); + var text = await response.Content.ReadAsStringAsync(); + response.IsSuccessStatusCode.ShouldBeTrue($"{method} {url} answered {(int)response.StatusCode}: {text}"); + return text; + } + + private async Task SendAsync(HttpMethod method, string url, Guid userId, Guid teamId, object? body = null) + { + using 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 (body != null) request.Content = JsonContent.Create(body); + + return await _factory.CreateClient().SendAsync(request); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Filters/FailuresAreAlwaysLoggedTests.cs b/backend/tests/CodeSpace.IntegrationTests/Filters/FailuresAreAlwaysLoggedTests.cs index ef27626fc..cd4c2a4de 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Filters/FailuresAreAlwaysLoggedTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Filters/FailuresAreAlwaysLoggedTests.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Shouldly; @@ -110,10 +111,27 @@ public void A_cancelled_request_is_not_recorded() logger.Entries.ShouldBeEmpty(); } - private static void Run(Exception exception, CapturingLogger logger) + /// The line names the request, and on a route whose path carries a bearer token that must be the route's template. + [Fact] + public void A_failure_on_a_bearer_token_route_is_recorded_with_the_route_template_not_the_token() + { + const string canary = "CANARY-filter-path-5d8a3c1b"; + var httpContext = new DefaultHttpContext(); + httpContext.Request.Method = "POST"; + httpContext.Request.Path = $"/api/invitations/{canary}/accept"; + httpContext.SetEndpoint(new RouteEndpoint(_ => Task.CompletedTask, RoutePatternFactory.Parse("api/invitations/{token}/accept"), 0, EndpointMetadataCollection.Empty, "accept")); + var logger = new CapturingLogger(); + + Run(new InvalidOperationException("thrown in a controller, outside Send"), logger, httpContext); + + logger.Entries.Single().Message.ShouldContain("POST /api/invitations/{token}/accept"); + logger.Entries.Single().Message.ShouldNotContain(canary); + } + + private static void Run(Exception exception, CapturingLogger logger, HttpContext? httpContext = null) { var filter = new GlobalExceptionFilter(logger); - var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + var actionContext = new ActionContext(httpContext ?? new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); var context = new ExceptionContext(actionContext, new List()) { Exception = exception }; filter.OnException(context); diff --git a/backend/tests/CodeSpace.IntegrationTests/Logging/BearerRouteLoggingTests.cs b/backend/tests/CodeSpace.IntegrationTests/Logging/BearerRouteLoggingTests.cs new file mode 100644 index 000000000..fa6de944e --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Logging/BearerRouteLoggingTests.cs @@ -0,0 +1,327 @@ +using System.Collections.Concurrent; +using CodeSpace.Api.Controllers; +using CodeSpace.Api.Logging; +using CodeSpace.Core.Settings.Application; +using CodeSpace.Core.Services.Workflows.Callbacks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Serilog; +using Serilog.Core; +using Serilog.Events; +using Serilog.Extensions.Logging; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Logging; + +/// +/// Invitation, password-reset and callback tokens travel in URL PATHS, and the request path is on every log line of the +/// request: in Serilog's completion line, and as RequestPath from the ASP.NET hosting scope on every event logged +/// while the request runs. For a route that binds a bearer parameter the path logged is the route's template instead. +/// These pin the template rewrite, its precedence over the hosting scope's property, and the operator-level setting the +/// one line it cannot reach depends on. Pure — they live here because UnitTests has no CodeSpace.Api reference. +/// +[Trait("Category", "Integration")] +public sealed class BearerRouteLoggingTests +{ + private const string Canary = "CANARY-bearer-route-7c4e1f9a2b"; + + [Theory] + [InlineData("api/invitations/{token}", "/api/invitations/" + Canary, "/api/invitations/{token}")] + [InlineData("api/invitations/{token}/accept", "/api/invitations/" + Canary + "/accept", "/api/invitations/{token}/accept")] + [InlineData("api/auth/reset-password/{token}", "/api/auth/reset-password/" + Canary, "/api/auth/reset-password/{token}")] + [InlineData("api/workflows/callbacks/{token:length(32)}", "/api/workflows/callbacks/" + Canary, "/api/workflows/callbacks/{token:length(32)}")] + [InlineData("api/workflows/runs/{runId:guid}", "/api/workflows/runs/0b5f2c1e-0000-4000-8000-000000000001", "/api/workflows/runs/0b5f2c1e-0000-4000-8000-000000000001")] + public void The_logged_path_is_the_route_template_only_where_the_route_binds_a_bearer_parameter(string template, string requestPath, string expected) + { + BearerRouteLogging.PathOf(RoutedContext(template, requestPath)).ShouldBe(expected); + } + + [Fact] + public void An_unrouted_request_logs_its_raw_path() + { + var context = new DefaultHttpContext(); + context.Request.Path = "/no/such/route"; + + BearerRouteLogging.PathOf(context).ShouldBe("/no/such/route"); + } + + [Theory] + [InlineData(Unrouted, "/api/workflows/callbacks/" + Canary, "/api/workflows/callbacks/{token}")] // before routing: UseHttpsRedirection's port warning + [InlineData(Fallback, "/api/workflows/callbacks/" + Canary, "/api/workflows/callbacks/{token}")] // GET or HEAD on a revealed callback URL + [InlineData(Fallback, "/API/Workflows/Callbacks/" + Canary + "/", "/API/Workflows/Callbacks/{token}")] // routing ignores case and a trailing slash + [InlineData(Fallback, "/api/invitations/" + Canary + "/accept", "/api/invitations/{token}/accept")] // GET on the accept route + [InlineData(Fallback, "/api/invitations/" + Canary + "/accept/more", "/api/invitations/{token}/accept/more")] // a path longer than any route + [InlineData(Fallback, "/invite/" + Canary, "/invite/{token}")] // the SPA's invite link opened on the API origin + [InlineData(Fallback, "/reset-password/" + Canary, "/reset-password/{token}")] // the SPA's reset link opened on the API origin + [InlineData(Rejected, "/api/auth/reset-password/" + Canary, "/api/auth/reset-password/{token}")] // a 405 rejection endpoint + [InlineData(Fallback, "/api/workflows/runs/" + Canary, "/api/workflows/runs/" + Canary)] // no bearer route has this shape + [InlineData("api/invitations/mine", "/api/invitations/mine", "/api/invitations/mine")] // a route that is not a bearer route answered it + public async Task A_request_no_bearer_route_answered_is_logged_with_its_bearer_path_segment_masked(string answeredBy, string requestPath, string expected) + { + // Redaction used to key only on the endpoint that answered. The same token reaches the fallback on a GET, a HEAD or + // the SPA's path, and reaches the log before routing has matched anything — so the path's SHAPE decides too. + var (sink, logger, app) = Pipeline(); + string? logged = null; + app.Run(context => + { + context.SetEndpoint(Answering(answeredBy)); + logger.LogWarning("after the endpoint was chosen"); + logged = BearerRouteLogging.PathOf(context); + return Task.CompletedTask; + }); + + var context = new DefaultHttpContext(); + context.Request.Method = "GET"; + context.Request.Path = requestPath; + using (logger.BeginScope(new Dictionary { ["RequestPath"] = requestPath })) + await app.Build()(context); + + logged.ShouldBe(expected); + sink.Events.Single().Properties["RequestPath"].ToString().ShouldBe($"\"{expected}\"", "the hosting scope's raw path on every event the request logs"); + if (expected != requestPath) sink.Rendered.ShouldNotContain(Canary); + } + + [Fact] + public async Task Every_bearer_route_whose_shape_the_path_has_masks_its_own_segment() + { + // Shapes are matched against the path as requested, not as an earlier shape masked it: a shorter route masking the + // segment a longer route expects as a literal must not hide the longer route's token. + var (_, _, app) = Pipeline(BearerEndpoint("hooks/{token}"), BearerEndpoint("hooks/inbound/{token}")); + string? logged = null; + app.Run(context => + { + logged = BearerRouteLogging.PathOf(context); + return Task.CompletedTask; + }); + + var context = new DefaultHttpContext(); + context.Request.Path = "/hooks/inbound/" + Canary; + await app.Build()(context); + + logged.ShouldBe("/hooks/{token}/{token}"); + } + + [Fact] + public async Task An_event_logged_before_routing_carries_the_masked_path() + { + // UseHttpsRedirection runs before UseRouting and warns once per process when it cannot find the https port; the + // first request of a fresh non-Development pod carried its path there with no endpoint chosen yet. + var (sink, logger, app) = Pipeline(); + app.Use(async (context, next) => + { + logger.LogWarning("Failed to determine the https port for redirect."); + await next(context); + }); + app.Run(_ => Task.CompletedTask); + + var context = new DefaultHttpContext(); + context.Request.Path = "/api/workflows/callbacks/" + Canary; + using (logger.BeginScope(new Dictionary { ["RequestPath"] = context.Request.Path.Value! })) + await app.Build()(context); + + sink.Events.Single().Properties["RequestPath"].ToString().ShouldBe("\"/api/workflows/callbacks/{token}\""); + sink.Rendered.ShouldNotContain(Canary); + } + + [Fact] + public void SpaBearerRoutes_are_the_paths_the_links_are_minted_on() + { + // The SPA's token routes are not this server's, so no endpoint names them; they are the prefixes PublicBaseUrlSetting + // mints links on (and the ones nginx masks). + var minted = new PublicBaseUrlSetting(new ConfigurationBuilder().AddInMemoryCollection(new Dictionary { [PublicBaseUrlSetting.ConfigurationKey] = "https://spa.example" }).Build()); + + BearerRouteLogging.SpaBearerRoutes.ShouldBe(["/invite/{token}", "/reset-password/{token}"]); + minted.InviteUrl(Canary).ShouldBe("https://spa.example/invite/" + Canary); + minted.PasswordResetUrl(Canary).ShouldBe("https://spa.example/reset-password/" + Canary); + } + + [Fact] + public void The_request_completion_line_carries_the_template_and_keeps_the_raw_path_elsewhere() + { + var bearer = BearerRouteLogging.RequestCompletionProperties(RoutedContext("api/invitations/{token}", "/api/invitations/" + Canary), "/api/invitations/" + Canary, 12.5, 200).ToDictionary(p => p.Name, p => p.Value.ToString()); + var plain = BearerRouteLogging.RequestCompletionProperties(RoutedContext("api/workflows/runs", "/api/workflows/runs"), "/api/workflows/runs", 1, 204).ToDictionary(p => p.Name, p => p.Value.ToString()); + + bearer["RequestPath"].ShouldBe("\"/api/invitations/{token}\""); + bearer.Keys.ShouldBe(["RequestMethod", "RequestPath", "StatusCode", "Elapsed"], ignoreOrder: true, "the same four properties Serilog's default completion line carries, so the message template keeps rendering"); + plain["RequestPath"].ShouldBe("\"/api/workflows/runs\"", "every other route logs the path Serilog handed in, unchanged"); + string.Concat(bearer.Values).ShouldNotContain(Canary); + } + + [Theory] + [InlineData("api/invitations/{token}/accept", "/api/invitations/" + Canary + "/accept", "/api/invitations/{token}/accept")] + [InlineData("api/workflows/runs/{runId:guid}", "/api/workflows/runs/0b5f2c1e-0000-4000-8000-000000000001", "/api/workflows/runs/0b5f2c1e-0000-4000-8000-000000000001")] + public async Task An_event_inside_the_endpoint_carries_the_template_over_the_hosting_scopes_raw_path_only_on_a_bearer_route(string template, string requestPath, string expected) + { + // The hosting scope's RequestPath reaches an event through Serilog's MEL provider, which adds scope properties only + // when absent. The middleware's LogContext enricher overwrites, so it wins whichever runs first. + var (sink, logger, app) = Pipeline(); + app.Run(_ => + { + logger.LogInformation("inside the endpoint"); + return Task.CompletedTask; + }); + + var context = RoutedContext(template, requestPath); + using (logger.BeginScope(new Dictionary { ["RequestPath"] = context.Request.Path.Value!, ["RequestId"] = "r-1" })) + await app.Build()(context); + + var logged = sink.Events.Single(); + logged.Properties["RequestPath"].ToString().ShouldBe($"\"{expected}\""); + logged.Properties["RequestId"].ToString().ShouldBe("\"r-1\"", "the rest of the hosting scope is untouched"); + sink.Rendered.ShouldNotContain(Canary); + } + + [Fact] + public async Task An_event_logged_the_moment_routing_matches_carries_the_template() + { + // Routing logs as it selects the endpoint (a body-size warning, its Debug "matched" line) — after the hosting scope + // was begun, before any middleware placed after UseRouting runs. Here the stand-in for routing sets the endpoint and + // logs straight away. + var (sink, logger, app) = Pipeline(); + app.Use(async (context, next) => + { + context.SetEndpoint(BearerEndpoint("api/workflows/callbacks/{token:length(32)}")); + logger.LogWarning("routing matched"); + await next(context); + }); + app.Run(_ => Task.CompletedTask); + + var context = new DefaultHttpContext(); + context.Request.Path = "/api/workflows/callbacks/" + Canary; + using (logger.BeginScope(new Dictionary { ["RequestPath"] = context.Request.Path.Value! })) + await app.Build()(context); + + sink.Events.Single().Properties["RequestPath"].ToString().ShouldBe("\"/api/workflows/callbacks/{token:length(32)}\""); + } + + [Fact] + public async Task Work_the_request_started_that_logs_after_it_ended_still_carries_the_template() + { + var (sink, logger, app) = Pipeline(); + var release = new TaskCompletionSource(); + Task? straggler = null; + app.Run(_ => + { + straggler = Task.Run(async () => + { + await release.Task; + logger.LogInformation("after the response"); + }); + return Task.CompletedTask; + }); + + var context = RoutedContext("api/auth/reset-password/{token}", "/api/auth/reset-password/" + Canary); + using (logger.BeginScope(new Dictionary { ["RequestPath"] = context.Request.Path.Value! })) + await app.Build()(context); + + context.SetEndpoint(null); // the server recycles the context once the request is over + release.SetResult(); + await straggler!; + + sink.Events.Single().Properties["RequestPath"].ToString().ShouldBe("\"/api/auth/reset-password/{token}\""); + } + + [Fact] + public void BearerParameters_is_pinned() + { + // Keyed on the route parameter NAME, so a new token route is covered by naming its parameter "token". + BearerRouteLogging.BearerParameters.ShouldBe(["token"]); + } + + [Fact] + public void The_callback_url_the_reveal_hands_out_is_the_path_the_callback_route_serves() + { + var controllerRoute = typeof(WorkflowCallbacksController).GetCustomAttributes(typeof(RouteAttribute), inherit: false).Cast().Single().Template; + var actionRoute = typeof(WorkflowCallbacksController).GetMethod(nameof(WorkflowCallbacksController.Resume))!.GetCustomAttributes(typeof(HttpPostAttribute), inherit: false).Cast().Single().Template; + + WorkflowCallbackUrlService.RoutePrefix.ShouldBe("/" + controllerRoute + "/"); + actionRoute.ShouldBe("{token:length(32)}", "the reveal appends the bare token, and the route binds it as 'token' — the name the log redaction keys on"); + } + + [Fact] + public void The_shipped_levels_keep_the_pre_routing_request_line_out_of_the_log() + { + // ASP.NET's own "Request starting POST https://host/api/invitations/" is logged before routing, so no route + // template exists to put in its place. It is Information under Microsoft.AspNetCore.Hosting.Diagnostics; the shipped + // override keeps it out. Lowering it would print every bearer token in a path — make that a reviewed decision. + var shipped = new ConfigurationBuilder().AddJsonFile(Path.Combine(FindBackendRoot(), "src", "CodeSpace.Api", "appsettings.json")).Build(); + var overrides = shipped.GetSection("Serilog:MinimumLevel:Override").GetChildren().ToDictionary(c => c.Key, c => c.Value); + + overrides["Microsoft"].ShouldBe("Warning"); + overrides.Keys.Where(k => k.StartsWith("Microsoft.AspNetCore", StringComparison.Ordinal)).ShouldBeEmpty("a narrower override could re-enable the hosting request line"); + } + + private static DefaultHttpContext RoutedContext(string template, string requestPath) + { + var context = new DefaultHttpContext(); + context.Request.Method = "POST"; + context.Request.Path = requestPath; + context.SetEndpoint(BearerEndpoint(template)); + + return context; + } + + private static RouteEndpoint BearerEndpoint(string template) => new(_ => Task.CompletedTask, RoutePatternFactory.Parse(template), 0, EndpointMetadataCollection.Empty, template); + + private const string Unrouted = "(nothing yet)"; + private const string Fallback = "{*path:nonfile}"; + private const string Rejected = "(405)"; + + /// The endpoint routing chose: none yet, MapFallback's catch-all, the method-rejection endpoint (not a RouteEndpoint), or a route by its template. + private static Endpoint? Answering(string answeredBy) => answeredBy switch + { + Unrouted => null, + Rejected => new Endpoint(_ => Task.CompletedTask, EndpointMetadataCollection.Empty, "405 HTTP Method Not Supported"), + _ => new RouteEndpoint(_ => Task.CompletedTask, RoutePatternFactory.Parse(answeredBy), answeredBy == Fallback ? int.MaxValue : 0, EndpointMetadataCollection.Empty, answeredBy), + }; + + /// The API's bearer routes as the host maps them, plus a route that is not one. + private static readonly Endpoint[] HostEndpoints = + [ + BearerEndpoint("api/invitations/{token}"), + BearerEndpoint("api/invitations/{token}/accept"), + BearerEndpoint("api/invitations/mine"), + BearerEndpoint("api/auth/reset-password/{token}"), + BearerEndpoint("api/workflows/callbacks/{token:length(32)}"), + BearerEndpoint("api/workflows/runs/{runId:guid}"), + ]; + + /// The redaction middleware in front of whatever the test adds, logging through Serilog's MEL provider the way the host does, over the host's endpoint table. + private static (CapturedEvents Sink, Microsoft.Extensions.Logging.ILogger Logger, ApplicationBuilder App) Pipeline() => Pipeline(HostEndpoints); + + private static (CapturedEvents Sink, Microsoft.Extensions.Logging.ILogger Logger, ApplicationBuilder App) Pipeline(params Endpoint[] hostEndpoints) + { + var sink = new CapturedEvents(); + var serilog = new LoggerConfiguration().MinimumLevel.Verbose().Enrich.FromLogContext().WriteTo.Sink(sink).CreateLogger(); + var logger = new SerilogLoggerFactory(serilog, dispose: true).CreateLogger("Request"); + var services = new ServiceCollection().AddSingleton(new DefaultEndpointDataSource(hostEndpoints)).BuildServiceProvider(); + var app = new ApplicationBuilder(services); + app.UseBearerRouteLogging(); + + return (sink, logger, app); + } + + private static string FindBackendRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "CodeSpace.sln"))) + directory = directory.Parent; + + return directory?.FullName ?? throw new DirectoryNotFoundException("Could not locate the CodeSpace backend root."); + } + + private sealed class CapturedEvents : ILogEventSink + { + public ConcurrentQueue Events { get; } = new(); + + public string Rendered => string.Join("\n", Events.Select(e => e.RenderMessage() + " " + string.Join(" ", e.Properties.Select(p => $"{p.Key}={p.Value}")))); + + public void Emit(LogEvent logEvent) => Events.Enqueue(logEvent); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/CallbackFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/CallbackFlowTests.cs index 63633e6f7..26ab9a8f9 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/CallbackFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/CallbackFlowTests.cs @@ -1,6 +1,8 @@ using System.Text.Json; using Autofac; using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Workflows.Callbacks; using CodeSpace.Core.Services.Workflows.Engine; using CodeSpace.IntegrationTests.Infrastructure; using CodeSpace.IntegrationTests.Workflows.Infrastructure; @@ -33,22 +35,29 @@ public class CallbackFlowTests public async Task Callback_token_resumes_the_run_and_surfaces_the_posted_body() { var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); - var workflowId = await CreateWorkflowAsync(teamId, userId, CallbackDefinition()); - var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); - - await RunEngineAsync(runId); + var runId = await ParkOnCallbackAsync(teamId, userId); - string token; + WorkflowRunWait wait; using (var verify = _fixture.BeginScope()) { var db = verify.Resolve(); (await db.WorkflowRun.AsNoTracking().SingleAsync(r => r.Id == runId)).Status.ShouldBe(WorkflowRunStatus.Suspended); - var wait = await db.WorkflowRunWait.AsNoTracking().SingleAsync(w => w.RunId == runId); + wait = await db.WorkflowRunWait.AsNoTracking().SingleAsync(w => w.RunId == runId); wait.WaitKind.ShouldBe(WorkflowWaitKinds.Callback); wait.WakeAt.ShouldBeNull("a callback wait has no timer — it wakes only on the external POST"); - token = wait.Token; } + // What an operator with runs.control reads off the run page. + var token = TokenOf(await RevealAsync(runId, wait.Id, teamId, userId)); + + token.Length.ShouldBe(32, "the callback route admits exactly {token:length(32)}"); + wait.TokenDigest.ShouldBe(WorkflowCallbackTokenSealer.Digest(token), "the digest a resume looks the wait up by"); + wait.TokenCiphertext.ShouldNotBeNull("the reveal reads the token back from its ciphertext"); + wait.TokenCiphertext.ShouldNotContain(token); + // Expand step: the token column keeps the token so a pod on the previous build, which compares it verbatim, still + // resumes this URL during the rollout and after a rollback. The follow-up release blanks it — flip this then. + wait.Token.ShouldBe(token); + // The external POST — the command the anonymous callbacks controller dispatches. bool resumed; using (var scope = _fixture.BeginScope()) @@ -73,6 +82,45 @@ public async Task Callback_token_resumes_the_run_and_surfaces_the_posted_body() } } + [Fact] + public async Task The_stored_digest_presented_as_a_token_does_not_resume() + { + // Whoever reads the row (a backup, a replica) holds the digest; it must not be a key to the wait it belongs to. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await ParkOnCallbackAsync(teamId, userId); + var wait = await LoadWaitAsync(runId); + + using var scope = _fixture.BeginScope(); + var resumed = await scope.Resolve().Send(new ResumeWorkflowCallbackCommand { Token = wait.TokenDigest!, Body = "{}" }); + + resumed.ShouldBeFalse(); + } + + [Fact] + public async Task Reveal_answers_404_for_a_run_outside_the_callers_team() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var (otherTeamId, _) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await ParkOnCallbackAsync(teamId, userId); + var wait = await LoadWaitAsync(runId); + + await Should.ThrowAsync(() => RevealAsync(runId, wait.Id, otherTeamId, userId)); + } + + [Fact] + public async Task Reveal_answers_404_once_the_wait_is_no_longer_pending() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await ParkOnCallbackAsync(teamId, userId); + var wait = await LoadWaitAsync(runId); + var token = TokenOf(await RevealAsync(runId, wait.Id, teamId, userId)); + + using (var scope = _fixture.BeginScope()) + (await scope.Resolve().Send(new ResumeWorkflowCallbackCommand { Token = token, Body = "{}" })).ShouldBeTrue(); + + await Should.ThrowAsync(() => RevealAsync(runId, wait.Id, teamId, userId)); + } + [Fact] public async Task Unknown_callback_token_does_not_resume() { @@ -87,6 +135,34 @@ public async Task Unknown_callback_token_does_not_resume() // ─── Helpers ──────────────────────────────────────────────────────────────── + private async Task ParkOnCallbackAsync(Guid teamId, Guid userId) + { + var workflowId = await CreateWorkflowAsync(teamId, userId, CallbackDefinition()); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + + await RunEngineAsync(runId); + + return runId; + } + + private async Task LoadWaitAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunWait.AsNoTracking().SingleAsync(w => w.RunId == runId); + } + + private async Task RevealAsync(Guid runId, Guid waitId, Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); + return await scope.Resolve().Send(new RevealWorkflowCallbackUrlCommand { RunId = runId, WaitId = waitId }); + } + + private static string TokenOf(WorkflowCallbackUrl url) + { + url.Path.ShouldStartWith(WorkflowCallbackUrlService.RoutePrefix); + return url.Path[WorkflowCallbackUrlService.RoutePrefix.Length..]; + } + private async Task CreateWorkflowAsync(Guid teamId, Guid userId, WorkflowDefinition definition) { using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/CallbackTokenDigestMigrationTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/CallbackTokenDigestMigrationTests.cs new file mode 100644 index 000000000..bbb5f2d6a --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/CallbackTokenDigestMigrationTests.cs @@ -0,0 +1,318 @@ +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Workflows.Callbacks; +using CodeSpace.Core.Services.Workflows.Engine; +using CodeSpace.IntegrationTests.Infrastructure; +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.Workflows; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// Migration 0248 is the EXPAND step for callback tokens: it adds token_digest and token_ciphertext, fills the +/// digest for every minted Callback token, and leaves token exactly as it was. A rolling deploy in any role order, +/// and a rollback to the previous build, must keep every issued callback URL working — and the digest must never become a +/// key anywhere. +/// +/// Rows are seeded the way each version writes them: the previous version (the token itself, nothing else), this +/// version's engine (token, digest and ciphertext), and the follow-up release that blanks the token (digest and +/// ciphertext only). "A previous-version pod" is : the callback lookup every +/// release before this change ran, verbatim, followed by the resume it hands the wait to. +/// +/// The replay commits on the shared fixture database. It only fills a digest a Callback row lacks, with the value +/// this version would write, so a sibling test's callback is unaffected. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class CallbackTokenDigestMigrationTests +{ + private const string MigrationFile = "0248_workflow_callback_token_digest.sql"; + + private readonly PostgresFixture _fixture; + + public CallbackTokenDigestMigrationTests(PostgresFixture fixture) { _fixture = fixture; } + + // ── The replay over rows the previous version wrote ────────────────────────── + + [Fact] + public async Task Replaying_0248_over_previous_version_rows_adds_the_digest_and_leaves_every_token_as_it_was() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSuspendedRunAsync(teamId, userId); + var shared = Guid.NewGuid().ToString("N"); + var pending = await AddPreviousVersionWaitAsync(runId, "cb", WorkflowWaitKinds.Callback, Guid.NewGuid().ToString("N")); + var resolved = await AddPreviousVersionWaitAsync(runId, "cb-done", WorkflowWaitKinds.Callback, Guid.NewGuid().ToString("N"), WorkflowWaitStatuses.Resolved); + var action = await AddPreviousVersionWaitAsync(runId, "act", WorkflowWaitKinds.Action, shared); + var timer = await AddPreviousVersionWaitAsync(runId, "sleep", WorkflowWaitKinds.Timer, shared); + var before = await LoadWaitsAsync(pending, resolved, action, timer); + + await ReplayMigrationAsync(); + var once = await LoadWaitsAsync(pending, resolved, action, timer); + + await ReplayMigrationAsync(); + var twice = await LoadWaitsAsync(pending, resolved, action, timer); + + foreach (var (row, original) in once.Zip(before)) + { + row.Token.ShouldBe(original.Token, "the token column is untouched: a previous-version pod compares it verbatim"); + row.TokenCiphertext.ShouldBeNull("SQL cannot reach the environment-held key"); + } + once[0].TokenDigest.ShouldBe(WorkflowCallbackTokenSealer.Digest(before[0].Token), "SQL's encode(sha256(...), 'hex') must equal the digest the application looks up"); + once[1].TokenDigest.ShouldBe(WorkflowCallbackTokenSealer.Digest(before[1].Token), "a closed callback gets its digest too, so the follow-up can blank every Callback token"); + once[2].TokenDigest.ShouldBeNull("an Action token is looked up verbatim by its authenticated verb"); + once[3].TokenDigest.ShouldBeNull(); + twice.Select(Snapshot).ShouldBe(once.Select(Snapshot), "a second run changes nothing"); + } + + [Fact] + public async Task A_url_issued_before_0248_resumes_on_this_version_and_can_still_be_revealed() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSuspendedRunAsync(teamId, userId); + var issued = Guid.NewGuid().ToString("N"); + var waitId = await AddPreviousVersionWaitAsync(runId, "cb", WorkflowWaitKinds.Callback, issued); + + await ReplayMigrationAsync(); + + (await RevealAsync(runId, waitId, teamId, userId)).Path.ShouldBe(WorkflowCallbackUrlService.RoutePrefix + issued, "the token column still holds it, so the run page can show the URL again"); + (await ResumeAsync(issued)).ShouldBeTrue("the URL an external system already holds keeps resuming the run"); + (await LoadWaitAsync(waitId)).Status.ShouldBe(WorkflowWaitStatuses.Resolved); + } + + // ── A previous-version pod against the migrated database ───────────────────── + + [Fact] + public async Task A_previous_version_pod_resumes_a_url_issued_before_0248_and_refuses_its_digest() + { + // A rolling deploy: the first new pod of either role has run 0248; an API pod on the previous build still serves. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSuspendedRunAsync(teamId, userId); + var issued = Guid.NewGuid().ToString("N"); + var waitId = await AddPreviousVersionWaitAsync(runId, "cb", WorkflowWaitKinds.Callback, issued); + + await ReplayMigrationAsync(); + var digest = (await LoadWaitAsync(waitId)).TokenDigest!; + + (await PreviousVersionResumeAsync(digest)).ShouldBeFalse("the digest a database reader holds must not be a key on a previous-version pod"); + (await PreviousVersionResumeAsync(issued)).ShouldBeTrue("the previous build still finds the issued token in the token column"); + (await LoadWaitAsync(waitId)).Status.ShouldBe(WorkflowWaitStatuses.Resolved); + } + + [Fact] + public async Task After_a_rollback_the_previous_version_shows_and_resumes_a_url_this_version_minted_and_refuses_its_digest() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await ParkOnCallbackAsync(teamId, userId); + var wait = await LoadCallbackWaitAsync(runId); + var issued = TokenOf(await RevealAsync(runId, wait.Id, teamId, userId)); + + wait.Token.ShouldBe(issued, "the previous build's run page builds the URL from the token column, so a rolled-back UI still shows the URL that works"); + wait.TokenDigest.ShouldBe(WorkflowCallbackTokenSealer.Digest(issued)); + (await PreviousVersionResumeAsync(wait.TokenDigest!)).ShouldBeFalse("the digest is not a key on the previous build either"); + (await PreviousVersionResumeAsync(issued)).ShouldBeTrue("a URL issued on this version keeps working after a rollback"); + (await LoadWaitAsync(wait.Id)).Status.ShouldBe(WorkflowWaitStatuses.Resolved); + } + + [Fact] + public async Task A_url_a_previous_version_pod_parks_after_0248_resumes_and_reveals_on_this_version() + { + // A previous-version worker during the rollout, or the whole fleet after a rollback, parks a callback the old way — + // no digest, no ciphertext. Rolling forward again must keep it working. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSuspendedRunAsync(teamId, userId); + var plaintext = Guid.NewGuid().ToString("N"); + var waitId = await AddPreviousVersionWaitAsync(runId, "cb", WorkflowWaitKinds.Callback, plaintext); + + (await RevealAsync(runId, waitId, teamId, userId)).Path.ShouldBe(WorkflowCallbackUrlService.RoutePrefix + plaintext); + (await ResumeAsync(plaintext)).ShouldBeTrue("this version also looks a minted-shape token up in the token column"); + } + + // ── Rolling the follow-up release back onto this one ───────────────────────── + + [Fact] + public async Task A_url_whose_token_a_later_release_blanked_resumes_by_its_digest_and_reveals_from_its_ciphertext() + { + // The follow-up release blanks the token column once every row has a digest and a ciphertext. Rolled back onto + // this version, such a row must still work — which is why this version already looks waits up by digest. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSuspendedRunAsync(teamId, userId); + var minted = Mint(); + var waitId = await AddBlankedCallbackWaitAsync(runId, minted, minted.Ciphertext); + + (await PreviousVersionResumeAsync(minted.Token)).ShouldBeFalse("precondition: nothing but the digest locates this row"); + (await RevealAsync(runId, waitId, teamId, userId)).Path.ShouldBe(WorkflowCallbackUrlService.RoutePrefix + minted.Token); + (await ResumeAsync(minted.Digest)).ShouldBeFalse("the digest presented as a token is not a key on this version"); + (await ResumeAsync(minted.Token)).ShouldBeTrue("this version resumes by the digest of the presented token"); + } + + [Fact] + public async Task A_row_with_neither_its_token_nor_a_ciphertext_still_resumes_but_cannot_be_revealed() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSuspendedRunAsync(teamId, userId); + var minted = Mint(); + var waitId = await AddBlankedCallbackWaitAsync(runId, minted, ciphertext: null); + + var refused = await Should.ThrowAsync(() => RevealAsync(runId, waitId, teamId, userId)); + refused.Kind.ShouldBe(CodeSpace.Messages.Failures.FailureKind.Conflict); + (await ResumeAsync(minted.Token)).ShouldBeTrue("the URL already handed out still resumes the run"); + } + + // ─── The previous version ─────────────────────────────────────────────────── + + /// + /// A callback POST on a pod running any build before this change: WorkflowResumeService.ResumeByCallbackTokenAsync + /// as of 270ce6c1d (unchanged since callbacks shipped) looks the wait up with + /// w.Token == token && w.Status == Pending && w.WaitKind == Callback — the SQL below, which reads + /// no column the previous build lacks — and hands the wait to ResumeCoreAsync(wait.RunId, body, wait.Id), which is + /// . Builds before a0eb90933 route any segment to it, so a + /// 64-character digest reaches this lookup there. + /// + private async Task PreviousVersionResumeAsync(string presented) + { + const string previousVersionLookup = "SELECT id, run_id FROM workflow_run_wait WHERE token = @token AND status = 'Pending' AND wait_kind = 'Callback' LIMIT 1"; + + await using var connection = new NpgsqlConnection(_fixture.ConnectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand(previousVersionLookup, connection); + command.Parameters.AddWithValue("token", presented); + await using var reader = await command.ExecuteReaderAsync(); + + if (!await reader.ReadAsync()) return false; + + var (waitId, runId) = (reader.GetGuid(0), reader.GetGuid(1)); + + using var scope = _fixture.BeginScope(); + return await scope.Resolve().ResumeWaitAsync(runId, waitId, """{"via":"previous-version"}""", CancellationToken.None); + } + + // ─── Helpers ──────────────────────────────────────────────────────────────── + + private async Task ReplayMigrationAsync() + { + var sql = await File.ReadAllTextAsync(Path.Combine(AppContext.BaseDirectory, "Persistence", "DbUpFiles", MigrationFile)); + + await using var connection = new NpgsqlConnection(_fixture.ConnectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand(sql, connection); + await command.ExecuteNonQueryAsync(); + } + + private async Task ResumeAsync(string token) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().Send(new ResumeWorkflowCallbackCommand { Token = token, Body = "{}" }); + } + + private async Task RevealAsync(Guid runId, Guid waitId, Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); + return await scope.Resolve().Send(new RevealWorkflowCallbackUrlCommand { RunId = runId, WaitId = waitId }); + } + + private static string TokenOf(WorkflowCallbackUrl url) + { + url.Path.ShouldStartWith(WorkflowCallbackUrlService.RoutePrefix); + return url.Path[WorkflowCallbackUrlService.RoutePrefix.Length..]; + } + + private SealedCallbackToken Mint() + { + using var scope = _fixture.BeginScope(); + return scope.Resolve().Mint(); + } + + private static (string Token, string? TokenDigest, string? TokenCiphertext, string Status) Snapshot(WorkflowRunWait w) => (w.Token, w.TokenDigest, w.TokenCiphertext, w.Status); + + private async Task LoadWaitAsync(Guid waitId) => (await LoadWaitsAsync(waitId)).Single(); + + private async Task> LoadWaitsAsync(params Guid[] waitIds) + { + using var scope = _fixture.BeginScope(); + var rows = await scope.Resolve().WorkflowRunWait.AsNoTracking().Where(w => waitIds.Contains(w.Id)).ToListAsync(); + + return waitIds.Select(id => rows.Single(w => w.Id == id)).ToList(); + } + + private async Task LoadCallbackWaitAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunWait.AsNoTracking().SingleAsync(w => w.RunId == runId && w.WaitKind == WorkflowWaitKinds.Callback); + } + + private async Task SeedSuspendedRunAsync(Guid teamId, Guid userId) + { + var workflowId = await CreateWorkflowAsync(teamId, userId, WorkflowsTestSeed.MinimalDefinition()); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + + using var scope = _fixture.BeginScope(); + await scope.Resolve().WorkflowRun.Where(r => r.Id == runId).ExecuteUpdateAsync(s => s.SetProperty(r => r.Status, WorkflowRunStatus.Suspended)); + + return runId; + } + + /// trigger → flow.wait_callback → terminal, walked by the real engine, which parks the wait and mints its token. + private async Task ParkOnCallbackAsync(Guid teamId, Guid userId) + { + var workflowId = await CreateWorkflowAsync(teamId, userId, new WorkflowDefinition + { + SchemaVersion = 1, + Nodes = new List + { + new() { Id = "start", TypeKey = "trigger.manual", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "callback", TypeKey = "flow.wait_callback", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "end", TypeKey = "builtin.terminal", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + }, + Edges = new List { new() { From = "start", To = "callback" }, new() { From = "callback", To = "end" } }, + }); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + + using var scope = _fixture.BeginScope(); + await scope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + + return runId; + } + + private async Task CreateWorkflowAsync(Guid teamId, Guid userId, WorkflowDefinition definition) + { + using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); + return await scope.Resolve().Send(new CreateWorkflowCommand + { + Name = "callback-digest-" + Guid.NewGuid().ToString("N")[..6], + Description = null, + Definition = definition, + Activations = new List(), + Enabled = true, + }); + } + + /// A wait row exactly as the previous version writes one: the token verbatim, no digest, no ciphertext. + private Task AddPreviousVersionWaitAsync(Guid runId, string nodeId, string waitKind, string token, string status = WorkflowWaitStatuses.Pending) => + AddWaitAsync(new WorkflowRunWait { RunId = runId, NodeId = nodeId, WaitKind = waitKind, Token = token, Status = status }); + + /// A Callback row as the follow-up release leaves it once it blanks the token column: digest and, when sealed, ciphertext. + private Task AddBlankedCallbackWaitAsync(Guid runId, SealedCallbackToken minted, string? ciphertext) => + AddWaitAsync(new WorkflowRunWait { RunId = runId, NodeId = "cb", WaitKind = WorkflowWaitKinds.Callback, Token = "", TokenDigest = minted.Digest, TokenCiphertext = ciphertext, Status = WorkflowWaitStatuses.Pending }); + + private async Task AddWaitAsync(WorkflowRunWait wait) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + wait.Id = Guid.NewGuid(); + wait.CreatedAt = DateTimeOffset.UtcNow; + db.WorkflowRunWait.Add(wait); + await db.SaveChangesAsync(); + + return wait.Id; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/ParallelWaitResumeScopeTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/ParallelWaitResumeScopeTests.cs index 25b1f1208..73b3f96bf 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/ParallelWaitResumeScopeTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/ParallelWaitResumeScopeTests.cs @@ -3,6 +3,7 @@ using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.Workflows; +using CodeSpace.Core.Services.Workflows.Callbacks; using CodeSpace.Core.Services.Workflows.Engine; using CodeSpace.IntegrationTests.Infrastructure; using CodeSpace.IntegrationTests.Workflows.Infrastructure; @@ -11,6 +12,7 @@ using CodeSpace.Messages.Constants; using CodeSpace.Messages.Dtos.Workflows; using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Workflows; using MediatR; using Microsoft.EntityFrameworkCore; using Shouldly; @@ -39,8 +41,12 @@ public async Task Callback_resume_resolves_only_its_own_wait_leaving_a_parallel_ var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); var runId = await SeedSuspendedRunAsync(teamId, userId); - const string callbackToken = "cb-tok-only-mine"; - var callbackWaitId = await AddWaitAsync(runId, "cb", WorkflowWaitKinds.Callback, callbackToken); + // Stored the way the engine parks a callback: the token, its digest and its ciphertext; the URL carries the token. + SealedCallbackToken callback; + using (var mint = _fixture.BeginScope()) + callback = mint.Resolve().Mint(); + var callbackToken = callback.Token; + var callbackWaitId = await AddWaitAsync(runId, "cb", WorkflowWaitKinds.Callback, callback.Token, callback); var approvalWaitId = await AddWaitAsync(runId, "appr", WorkflowWaitKinds.Approval, token: ""); bool resumed; @@ -107,7 +113,7 @@ await db.WorkflowRun.Where(r => r.Id == runId) return runId; } - private async Task AddWaitAsync(Guid runId, string nodeId, string waitKind, string token) + private async Task AddWaitAsync(Guid runId, string nodeId, string waitKind, string token, SealedCallbackToken? sealedCallback = null) { using var scope = _fixture.BeginScope(); var db = scope.Resolve(); @@ -120,6 +126,8 @@ private async Task AddWaitAsync(Guid runId, string nodeId, string waitKind NodeId = nodeId, WaitKind = waitKind, Token = token, + TokenDigest = sealedCallback?.Digest, + TokenCiphertext = sealedCallback?.Ciphertext, Status = WorkflowWaitStatuses.Pending, CreatedAt = DateTimeOffset.UtcNow, }); diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowCallbackTokenSealerTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowCallbackTokenSealerTests.cs new file mode 100644 index 000000000..61727badf --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowCallbackTokenSealerTests.cs @@ -0,0 +1,88 @@ +using System.Security.Cryptography; +using System.Text; +using CodeSpace.Core.Services.Variables; +using CodeSpace.Core.Services.Workflows.Callbacks; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// A callback wait's token is a bearer credential for an anonymous route. A wait row keeps it three ways: the token as +/// minted (what a pod on the previous version compares verbatim, kept until a later release blanks it), its SHA-256 (what +/// a resume looks it up by, and the only lookup form left once the token is blanked) and its AES-GCM ciphertext under the +/// deployment's environment-held key (for the RunsControl-gated reveal). These pin the three forms a minted token takes, +/// which presented values may match the plaintext column, which rows a reveal can turn back into a URL, and that only the +/// deployment's key opens the ciphertext. +/// +[Trait("Category", "Unit")] +public sealed class WorkflowCallbackTokenSealerTests +{ + private const string MintedShape = "0123456789abcdef0123456789abcdef"; + + private static readonly WorkflowCallbackTokenSealer Sealer = new(new AesGcmVariableEncryption(RandomNumberGenerator.GetBytes(32))); + + [Fact] + public void A_minted_token_has_the_shape_the_callback_route_admits_with_its_digest_and_an_opaque_ciphertext() + { + var minted = Sealer.Mint(); + + minted.Token.Length.ShouldBe(32, "WorkflowCallbacksController admits exactly {token:length(32)}"); + minted.Token.ShouldMatch("^[0-9a-f]{32}$", "the shape every earlier version minted, so a previous-version pod compares it verbatim"); + minted.Digest.ShouldBe(Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(minted.Token)))); + minted.Ciphertext.ShouldNotContain(minted.Token, customMessage: "the reveal copy is encrypted, never the token as text"); + Encoding.UTF8.GetString(Convert.FromBase64String(minted.Ciphertext)).ShouldNotContain(minted.Token, customMessage: "nor the token's bytes under an encoding"); + } + + [Fact] + public void The_digest_is_lowercase_hex_sha256_so_it_equals_what_the_migration_writes_in_sql() + { + // encode(sha256(convert_to('abc', 'UTF8')), 'hex') — the FIPS 180-2 "abc" vector, in the case Postgres prints. + WorkflowCallbackTokenSealer.Digest("abc").ShouldBe("ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"); + } + + [Theory] + [InlineData(MintedShape, true)] // what every version mints and keeps in the token column + [InlineData("0123456789ABCDEF0123456789ABCDEF", false)] // never minted (Guid "N" is lowercase) + [InlineData("ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", false)] // a stored digest presented as a token + [InlineData("", false)] // the blank a later release writes over the token + [InlineData("CANARY-not-a-minted-token", false)] + public void Only_a_minted_shape_token_is_looked_up_in_the_plaintext_column(string presented, bool matchesPlaintext) + { + WorkflowCallbackTokenSealer.PlaintextFormOf(presented).ShouldBe(matchesPlaintext ? presented : null, "a presented value is compared with the token column only when a version could have stored it there — so a digest, or a blanked column, is never a key"); + } + + [Fact] + public void A_reveal_opens_the_ciphertext_even_once_the_plaintext_column_is_blanked() + { + var minted = Sealer.Mint(); + + Sealer.Open(minted.Token, minted.Ciphertext).ShouldBe(minted.Token, "the reveal opens the exact token the URL needs"); + Sealer.Open(storedToken: "", minted.Ciphertext).ShouldBe(minted.Token, "the follow-up release blanks the token column; the ciphertext alone still reveals the URL"); + } + + [Fact] + public void A_row_that_holds_only_its_plaintext_reveals_it() + { + // A row minted before 0248, or parked by a previous-version pod during the rollout or after a rollback: no ciphertext. + Sealer.Open(MintedShape, ciphertext: null).ShouldBe(MintedShape); + } + + [Theory] + [InlineData("")] // blanked by a later release, never sealed + [InlineData("ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad")] // a digest is never handed out as a URL + public void A_row_with_neither_a_ciphertext_nor_a_minted_token_cannot_be_revealed(string storedToken) + { + Sealer.Open(storedToken, ciphertext: null).ShouldBeNull(); + } + + [Fact] + public void A_ciphertext_opens_only_with_the_deployment_key_it_was_sealed_under() + { + // The key lives in the deployment's environment, never in the database — a database or backup reader holds the + // ciphertext and still cannot open it. Any other key fails closed rather than returning a wrong token. + var minted = Sealer.Mint(); + var otherDeployment = new WorkflowCallbackTokenSealer(new AesGcmVariableEncryption(RandomNumberGenerator.GetBytes(32))); + + Should.Throw(() => otherDeployment.Open("", minted.Ciphertext)); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowWaitKindsTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowWaitKindsTests.cs index ae92bcf46..e5a0c94f9 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowWaitKindsTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/WorkflowWaitKindsTests.cs @@ -74,4 +74,54 @@ public void IsOperatorReissuable_allows_only_the_signal_driven_stranding_kinds(s [Fact] public void IsOperatorReissuable_is_fail_closed_for_an_unknown_kind() => WorkflowWaitKinds.IsOperatorReissuable("SomeFutureKind").ShouldBeFalse("a new wait kind is NOT operator-reissuable until deliberately added to the allow-list"); + + // ── HasBearerToken: whose token alone resolves the wait ─────────────────── + // Only a Callback is resumed by presenting its token with no session (the anonymous callback route), so only its token + // is withheld from every projection and stored as a digest. Every other kind's token is a run id or needs runs.decide. + // Every known kind is listed so a new one is a reviewed decision here. + + [Theory] + [InlineData(WorkflowWaitKinds.Callback, true)] + [InlineData(WorkflowWaitKinds.Timer, false)] + [InlineData(WorkflowWaitKinds.Approval, false)] + [InlineData(WorkflowWaitKinds.Action, false)] + [InlineData(WorkflowWaitKinds.Decision, false)] + [InlineData(WorkflowWaitKinds.Subworkflow, false)] + [InlineData(WorkflowWaitKinds.AgentRun, false)] + [InlineData(WorkflowWaitKinds.SupervisorDecision, false)] + [InlineData(WorkflowWaitKinds.SupervisorAgentWaits, false)] + [InlineData(WorkflowWaitKinds.SupervisorInfraPark, false)] + public void HasBearerToken_is_true_only_for_the_anonymously_resumed_kind(string waitKind, bool expected) => + WorkflowWaitKinds.HasBearerToken(waitKind).ShouldBe(expected); + + // ── ProjectedToken: what a run read shows in place of a bearer token ───── + // A read never carries a Callback's token. It carries a fixed placeholder instead of null because every SPA build + // since the pending-wait decoder shipped (frontend/src/api/workflows.ts, decodeWorkflowRunPendingWaitObservation) + // refuses a wait whose token is not a 1..128-character string — a null showed "Pending action unavailable" on a + // previous-version SPA during the deploy. frontend/src/api/workflowRunPendingWaitApi.test.ts decodes this literal. + + [Theory] + [InlineData(WorkflowWaitKinds.Callback, WorkflowWaitKinds.WithheldBearerToken)] + [InlineData(WorkflowWaitKinds.Timer, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.Approval, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.Action, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.Decision, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.Subworkflow, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.AgentRun, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.SupervisorDecision, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.SupervisorAgentWaits, "0123456789abcdef0123456789abcdef")] + [InlineData(WorkflowWaitKinds.SupervisorInfraPark, "0123456789abcdef0123456789abcdef")] + public void ProjectedToken_withholds_only_a_bearer_token(string waitKind, string expected) => + WorkflowWaitKinds.ProjectedToken(waitKind, "0123456789abcdef0123456789abcdef").ShouldBe(expected); + + [Fact] + public void WithheldBearerToken_is_a_value_every_spa_decodes_and_no_callback_route_can_resume_with() + { + // Pinned: the frontend test decodes this exact literal through the decoder every SPA build ships. + WorkflowWaitKinds.WithheldBearerToken.ShouldBe("withheld-reload-the-page-to-reveal"); + + WorkflowWaitKinds.WithheldBearerToken.Length.ShouldBeInRange(1, 128, "the SPA decoder's bounds for a wait token"); + WorkflowWaitKinds.WithheldBearerToken.Length.ShouldNotBe(32, "this version's callback route admits only {token:length(32)}"); + WorkflowWaitKinds.WithheldBearerToken.ShouldNotMatch("^[0-9a-f]+$", "never a shape any version mints, so no wait row's token can equal it — not even on an older build whose route admits any segment"); + } } diff --git a/frontend/docker/e2e/run.sh b/frontend/docker/e2e/run.sh index 99b3a5397..5694a79ef 100755 --- a/frontend/docker/e2e/run.sh +++ b/frontend/docker/e2e/run.sh @@ -2,7 +2,9 @@ # SPA image E2E: build the REAL frontend image and start it the way the cluster does (the ConfigMap's .env mounted at # /usr/share/nginx/html/.env, the image's own CMD /init.sh), then prove over HTTP what static-hosting.test.ts can only # pin as source text: init.sh derives the API origin from the substituted env, nginx accepts the config and reads that -# origin, and every response carries the CSP, nosniff and Referrer-Policy, with /env.js handing the SPA its env. +# origin, and every response carries the CSP, nosniff and Referrer-Policy, with /env.js handing the SPA its env. It also +# reads the container log back: a token in an invitation, reset or callback path, or in a Referer naming one, is logged as +# {token}, and every other request verbatim. # # Fidelity: high. The real Dockerfile, init.sh, @import-meta-env/cli (fetched by npx at start, as in production), # csp-api-origin.mjs and nginx. The expected CSP is nginx.conf's own header with the derived source filled in, so the @@ -110,6 +112,37 @@ expect_served() { # $1 = case, $2 = VITE_API_URL, $3 = the CSP source init.sh mu echo " CSP, nosniff and Referrer-Policy on every response; /env.js carries the env" } +expect_access_log_redacts() { # the request line and the Referer of a token route reach the log as {token}; others verbatim + local name port base logs logged canary="CANARY-spa-log-$$-$(date +%s)" + + echo "==> access-log: token paths and Referers are logged as {token}, every other request verbatim" + name="$(start access-log "")" + port="$(wait_serving "$name")" + base="http://127.0.0.1:$port" + + for path in "/invite/$canary" "/reset-password/$canary" "/api/workflows/callbacks/$canary"; do + curl -fsS -o /dev/null "$base$path" || fail "access-log: GET $path failed" "$name" + done + curl -fsS -o /dev/null -H "Referer: $base/invite/$canary" "$base/env.js" || fail "access-log: GET /env.js with a token link as its Referer failed" "$name" + curl -fsS -o /dev/null "$base/teams/demo/runs" || fail "access-log: GET /teams/demo/runs failed" "$name" + + # nginx writes each line as the request ends; the last request's line marks the log complete. + for _ in $(seq 1 20); do + logs="$(docker logs "$name" 2>&1)" + case "$logs" in *'"GET /teams/demo/runs HTTP/1.1"'*) break ;; esac + sleep 0.5 + done + + case "$logs" in *"$canary"*) fail "access-log: a token reached the container log: $(printf '%s\n' "$logs" | grep -F "$canary")" "$name" ;; esac + + for logged in '"GET /invite/{token} HTTP/1.1"' '"GET /reset-password/{token} HTTP/1.1"' '"GET /api/workflows/callbacks/{token} HTTP/1.1"' "\"$base/invite/{token}\"" '"GET /teams/demo/runs HTTP/1.1"'; do + case "$logs" in *"$logged"*) ;; *) fail "access-log: no $logged in the container log — is the server still logging with nginx.conf's codespace format? ('docker logs $name')" "$name" ;; esac + done + + docker rm -f "$name" >/dev/null 2>&1 || true + echo " token paths and Referers logged as {token}; other requests verbatim" +} + expect_refused() { # $1 = case, $2 = VITE_API_URL init.sh must stop on, naming the setting, before nginx starts local name state="" logs @@ -140,5 +173,6 @@ expect_served api-elsewhere "https://api.example.test/base" "https://api.example expect_served api-path "/backend" "" expect_served api-protocol-relative "//api.example.test:8443/base" "api.example.test:8443" expect_refused api-websocket "wss://api.example.test" +expect_access_log_redacts -echo "PASS: the SPA image serves its CSP with the API origin init.sh derived, plus nosniff and Referrer-Policy" +echo "PASS: the SPA image serves its CSP with the API origin init.sh derived, plus nosniff and Referrer-Policy, and logs no path token" diff --git a/frontend/nginx.conf b/frontend/nginx.conf index c17eeb827..6ff1bb0b4 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -1,8 +1,29 @@ +# Invitation, password-reset and workflow-callback links carry a bearer token as a path segment. The image's default +# `main` access log writes the request line ($request) and the Referer, and a page opened at /invite/ sends that +# whole URL as the Referer of every asset it loads, so the token would land in the container log, readable by anyone who +# can read logs. These maps put `{token}` in place of that segment in the copy the log gets; every other request is +# logged exactly as `main` logs it. static-hosting.test.ts pins the routes, docker/e2e/run.sh the served log. +map $request_uri $codespace_logged_uri { + "~*^(?/(?:invite|reset-password|api/invitations|api/auth/reset-password|api/workflows/callbacks)/)[^/?]*(?.*)$" "${logged_uri_head}{token}${logged_uri_tail}"; + default $request_uri; +} + +map $http_referer $codespace_logged_referer { + "~*^(?[^/]*//[^/]*)(?/(?:invite|reset-password|api/invitations|api/auth/reset-password|api/workflows/callbacks)/)[^/?]*(?.*)$" "${logged_referer_origin}${logged_referer_head}{token}${logged_referer_tail}"; + default $http_referer; +} + +log_format codespace '$remote_addr - $remote_user [$time_local] "$request_method $codespace_logged_uri $server_protocol" ' + '$status $body_bytes_sent "$codespace_logged_referer" ' + '"$http_user_agent" "$http_x_forwarded_for"'; + server { listen 80; listen [::]:80; server_name localhost; + access_log /var/log/nginx/access.log codespace; + # Sets $codespace_api_origin — the API origin the SPA was configured with ("" when same-origin). Written at # container start by docker/init.sh from the substituted env.js; nginx refuses to start without it. include /etc/nginx/codespace/api-origin.conf; diff --git a/frontend/src/api/workflowRunPendingWaitApi.test.ts b/frontend/src/api/workflowRunPendingWaitApi.test.ts index a7e10435a..96e535b92 100644 --- a/frontend/src/api/workflowRunPendingWaitApi.test.ts +++ b/frontend/src/api/workflowRunPendingWaitApi.test.ts @@ -34,6 +34,28 @@ describe("Workflow Run pending-wait API", () => { await expect(workflowsApi.getRunPendingWait(runId)).rejects.toBeInstanceOf(InvalidWorkflowRunPendingWaitObservationError); }); + it("reads a Callback wait whose token the server withholds — the placeholder passes the decoder every SPA build ships", async () => { + // The literal the API sends in place of a Callback's token (WorkflowWaitKinds.WithheldBearerToken, pinned by + // WorkflowWaitKindsTests). A null here showed "Pending action unavailable" on an SPA built before the reveal existed. + const withheld = "withheld-reload-the-page-to-reveal"; + vi.stubGlobal("fetch", vi.fn(() => json({ runId, wait: { id: waitId, nodeId: "cb", kind: "Callback", token: withheld, wakeAt: null, promptState: "Missing", promptPrefix: null } }))); + + const result = await workflowsApi.getRunPendingWait(runId); + + expect(result?.wait).toEqual({ id: waitId, nodeId: "cb", kind: "Callback", token: withheld, wakeAt: null, promptState: "Missing", promptPrefix: null }); + }); + + it("reveals a callback URL through the runs.control-gated command", async () => { + vi.stubGlobal("fetch", vi.fn(() => json({ waitId, path: "/api/workflows/callbacks/0123456789abcdef0123456789abcdef" }))); + + const revealed = await workflowsApi.revealCallbackUrl(runId, waitId); + + expect(revealed).toEqual({ waitId, path: "/api/workflows/callbacks/0123456789abcdef0123456789abcdef" }); + const [url, init] = vi.mocked(fetch).mock.calls[0] as [string, RequestInit]; + expect(String(url).endsWith(`/api/workflows/runs/${runId}/waits/${waitId}/callback-url`)).toBe(true); + expect(init.method).toBe("POST"); + }); + it("conflates an inaccessible run with missing", async () => { vi.stubGlobal("fetch", vi.fn(() => json({ title: "Not Found" }, 404))); await expect(workflowsApi.getRunPendingWait(runId)).resolves.toBeNull(); diff --git a/frontend/src/api/workflows.ts b/frontend/src/api/workflows.ts index de191c816..7f881b7ca 100644 --- a/frontend/src/api/workflows.ts +++ b/frontend/src/api/workflows.ts @@ -391,10 +391,16 @@ export interface WorkflowRunNodeSummary { /** The outstanding wait a Suspended run is parked on — drives the resume affordance. */ export interface WorkflowRunWaitInfo { + /** The wait row's id — what the reissue verb and the callback-URL reveal address. */ + id: string; nodeId: string; /** "Timer" | "Approval" | "Callback" | "Subworkflow". */ kind: string; - /** Correlation token — for a Callback wait, the secret the callback URL is built from. */ + /** + * Correlation token — a Subworkflow wait's child run id. Never a Callback wait's token: its token alone resumes the run + * without a session, so the server sends a placeholder instead and only `workflowsApi.revealCallbackUrl` + * (runs.control) hands the token out. + */ token: string; /** When the scheduled resume fires (Timer only). */ wakeAt?: string | null; @@ -402,6 +408,12 @@ export interface WorkflowRunWaitInfo { payload?: unknown; } +/** A pending Callback wait's URL path (`/api/workflows/callbacks/{token}`). Whoever holds it resumes the run without signing in. */ +export interface WorkflowCallbackUrl { + waitId: string; + path: string; +} + export type WorkflowRunPendingWaitPromptState = "Missing" | "Exact" | "Truncated" | "Invalid"; /** Bounded action descriptor; the raw wait payload and run graph never cross this read seam. */ @@ -414,6 +426,7 @@ export interface WorkflowRunPendingWait { id: string; nodeId: string; kind: string; + /** A placeholder for a Callback wait — see `WorkflowRunWaitInfo.token`. */ token: string; wakeAt: string | null; promptState: WorkflowRunPendingWaitPromptState; @@ -1682,6 +1695,10 @@ export const workflowsApi = { body: JSON.stringify(body), }), + /** The URL of a pending Callback wait. Needs runs.control (a Viewer gets 403); the server logs who revealed it. */ + revealCallbackUrl: (runId: string, waitId: string) => + fetchJson(`/api/workflows/runs/${runId}/waits/${waitId}/callback-url`, { method: "POST" }), + /** Continue a STRANDED Suspended run (no pending wait) on demand — drives the same re-dispatch the reconciler does. Returns whether this call drove it. */ continueRun: (runId: string) => fetchJson<{ continued: boolean }>(`/api/workflows/runs/${runId}/continue`, { method: "POST" }), diff --git a/frontend/src/components/workflows/RunDetailView.test.tsx b/frontend/src/components/workflows/RunDetailView.test.tsx index dc6572aac..54eed95fd 100644 --- a/frontend/src/components/workflows/RunDetailView.test.tsx +++ b/frontend/src/components/workflows/RunDetailView.test.tsx @@ -177,7 +177,7 @@ describe("RunDetailView — sub-workflow step drill-down", () => { useWorkflowRunMock.mockImplementation((runId: string) => { if (runId === "parent-1") return ok(detail({ status: "Suspended", - pendingWait: { nodeId: "sub", kind: "Subworkflow", token: "child-1", payload: {} }, + pendingWait: { id: "wait-sub", nodeId: "sub", kind: "Subworkflow", token: "child-1", payload: {} }, nodes: [node({ nodeId: "sub", status: "Suspended", childRunId: "child-1" })], })); if (runId === "child-1") return ok(detail({ id: "child-1", nodes: [node({ nodeId: "child-step" })] })); diff --git a/frontend/src/components/workflows/RunDetailView.tsx b/frontend/src/components/workflows/RunDetailView.tsx index 20dd0bd27..3b048526d 100644 --- a/frontend/src/components/workflows/RunDetailView.tsx +++ b/frontend/src/components/workflows/RunDetailView.tsx @@ -6,8 +6,9 @@ import { adaptWorkflowRunViewToCanvas, type WorkflowRunViewAvailability, type Wo import type { NodeManifestDto, WorkflowRunDetail, WorkflowRunIdentity, WorkflowRunNodeSummary, WorkflowRunPendingWait, WorkflowRunWaitInfo } from "@/api/workflows"; import { ApiError } from "@/api/request"; import { useAgentRun } from "@/hooks/use-agents"; +import { TeamPermissions, useTeamPermissions } from "@/hooks/use-team-management"; import { useWorkflowRunViewMetadata } from "@/hooks/use-workflow-run-view-metadata"; -import { isRunActive, useNodeManifests, useResumeRun, useWorkflowRunDetail, useWorkflowRunIdentity, useWorkflowRunPendingWait } from "@/hooks/use-workflows"; +import { isRunActive, useNodeManifests, useResumeRun, useRevealCallbackUrl, useWorkflowRunDetail, useWorkflowRunIdentity, useWorkflowRunPendingWait } from "@/hooks/use-workflows"; import { AgentRunTimeline } from "./AgentRunTimeline"; import { AgentToolCalls } from "./AgentToolCalls"; @@ -440,7 +441,7 @@ function hasContent(value: unknown): boolean { /** * The resume affordance for a Suspended run. An Approval wait gets approve/reject + an optional * comment (posts the decision, then the live poll shows the run continue). A Timer wait just - * shows when it'll wake. A Callback wait shows the tokened URL. A Subworkflow wait embeds the live + * shows when it'll wake. A Callback wait reveals its URL on request. A Subworkflow wait embeds the live * child run inline — including ITS resume affordance, so e.g. an approval deep inside the child is * operable right here; resolving it completes the child, which auto-resumes this run. */ @@ -491,21 +492,58 @@ export function SuspendedPanel({ runId, wait, depth = 0, onOpenRun }: { runId: s ); } - if (wait.kind === "Callback") { - const url = `${window.location.origin}/api/workflows/callbacks/${wait.token}`; - return ( -
-

Waiting for callback

-
An external system resumes this run by POSTing to:
+ // Keyed by the wait, so a revealed URL never outlives it: when the poll moves to another callback wait while the run + // stays Suspended (parallel branches, a loop re-parking), the panel starts over instead of showing a dead URL. + if (wait.kind === "Callback") return ; + + return null; +} + +/** The shape every API build mints a callback token in. The placeholder an API sends in a token's place never has it. */ +const MINTED_CALLBACK_TOKEN = /^[0-9a-f]{32}$/; + +/** + * The URL an API from before the reveal existed projects: it hands every member the token itself and has no reveal route. + * Such an API serves this page when the page is deployed first, or when the API alone is rolled back; showing its URL keeps + * the panel working there as it always had. Every later API sends a placeholder, so this is null. + */ +function projectedCallbackUrl(projectedToken: string): string | null { + return MINTED_CALLBACK_TOKEN.test(projectedToken) ? new URL(`/api/workflows/callbacks/${projectedToken}`, window.location.origin).toString() : null; +} + +/** + * A Callback wait's URL resumes the run without a session, so no run read carries it: a member who can control runs + * reveals it on demand (the server logs who did), and everyone else is told who can. + */ +function CallbackWaitPanel({ runId, waitId, projectedToken }: { runId: string; waitId: string; projectedToken: string }) { + const mayControl = useTeamPermissions().can(TeamPermissions.RunsControl); + const reveal = useRevealCallbackUrl(runId); + const [url, setUrl] = useState(() => projectedCallbackUrl(projectedToken)); + + // A refused reveal renders from reveal.error below, so its rejection needs no handler of its own. + const show = () => reveal.mutateAsync(waitId).then((revealed) => setUrl(new URL(revealed.path, window.location.origin).toString()), () => undefined); + const hide = () => { setUrl(null); reveal.reset(); }; + + return ( +
+

Waiting for callback

+
An external system resumes this run by POSTing to its callback URL. Anyone holding the URL can resume the run, so it is shown only on request.
+ {url != null ? (
e.currentTarget.select()} /> +
-
- ); - } - - return null; + ) : mayControl ? ( +
+ +
+ ) : ( +
Only members who can control runs can reveal it.
+ )} + {reveal.error instanceof ApiError &&
{reveal.error.message}
} +
+ ); } /** diff --git a/frontend/src/components/workflows/SuspendedPanel.test.tsx b/frontend/src/components/workflows/SuspendedPanel.test.tsx index 0d6678847..33b06de40 100644 --- a/frontend/src/components/workflows/SuspendedPanel.test.tsx +++ b/frontend/src/components/workflows/SuspendedPanel.test.tsx @@ -12,10 +12,16 @@ import { SuspendedPanel } from "./RunDetailView"; * 3. a Timer wait shows a wake hint, NOT approval buttons. */ -const { mutate, useWorkflowRunMock } = vi.hoisted(() => ({ mutate: vi.fn(), useWorkflowRunMock: vi.fn() })); +const { mutate, useWorkflowRunMock, reveal, permissions } = vi.hoisted(() => ({ + mutate: vi.fn(), + useWorkflowRunMock: vi.fn(), + reveal: { mutateAsync: vi.fn(), reset: vi.fn(), isPending: false, error: null as unknown }, + permissions: { held: new Set() }, +})); vi.mock("@/hooks/use-workflows", () => ({ useResumeRun: () => ({ mutate, isPending: false, isError: false }), + useRevealCallbackUrl: () => reveal, isRunActive: (status: string) => !["Success", "Failure", "Cancelled"].includes(status), // Used by the embedded child RunDetailView for a Subworkflow wait. useWorkflowRun: (runId: string) => useWorkflowRunMock(runId), @@ -26,6 +32,11 @@ vi.mock("@/hooks/use-workflows", () => ({ useRunPhases: () => ({ data: undefined }), // the embedded child is nested → phases query is disabled anyway })); +vi.mock("@/hooks/use-team-management", () => ({ + TeamPermissions: { RunsControl: "runs.control" }, + useTeamPermissions: () => ({ can: (permission: string) => permissions.held.has(permission), role: null, isPersonal: false }), +})); + vi.mock("@/hooks/use-workflow-run-view-metadata", () => ({ useWorkflowRunViewMetadata: (runId: string) => ({ isLoading: false, error: null, data: { runId, runNumber: 1, workflowId: "w", workflowVersion: 1, sourceType: "workflow.child", parentRunId: null, @@ -45,9 +56,12 @@ vi.mock("@/hooks/use-agents", () => ({ })); function approvalWait(prompt: string): WorkflowRunWaitInfo { - return { nodeId: "approval", kind: "Approval", token: "tok-1", payload: { prompt } }; + return { id: "wait-a", nodeId: "approval", kind: "Approval", token: "tok-1", payload: { prompt } }; } +// A run read never carries a Callback's token: the API sends this placeholder in its place (WorkflowWaitKinds.WithheldBearerToken). +const callbackWait: WorkflowRunWaitInfo = { id: "wait-cb", nodeId: "cb", kind: "Callback", token: "withheld-reload-the-page-to-reveal", payload: {} }; + describe("SuspendedPanel", () => { it("approves with the typed comment", () => { render(); @@ -70,20 +84,63 @@ describe("SuspendedPanel", () => { }); it("shows a wake hint for a Timer wait — no approval buttons", () => { - const wait: WorkflowRunWaitInfo = { nodeId: "sleep", kind: "Timer", token: "tok-2", wakeAt: "2026-05-30T11:00:00Z", payload: {} }; + const wait: WorkflowRunWaitInfo = { id: "wait-t", nodeId: "sleep", kind: "Timer", token: "tok-2", wakeAt: "2026-05-30T11:00:00Z", payload: {} }; render(); expect(screen.queryByText("Approve")).toBeNull(); expect(screen.getByText(/Resumes around/)).toBeTruthy(); }); - it("shows the tokened callback URL for a Callback wait — no approval buttons", () => { - const wait: WorkflowRunWaitInfo = { nodeId: "cb", kind: "Callback", token: "abc123", payload: {} }; - render(); + it("hides the callback URL until a member who can control runs reveals it — then shows the URL the run resumes on", async () => { + permissions.held = new Set(["runs.control"]); + reveal.mutateAsync.mockResolvedValueOnce({ waitId: "wait-cb", path: "/api/workflows/callbacks/0123456789abcdef0123456789abcdef" }); + render(); expect(screen.queryByText("Approve")).toBeNull(); - const url = screen.getByDisplayValue(/\/api\/workflows\/callbacks\/abc123$/); - expect(url).toBeTruthy(); + expect(screen.queryByDisplayValue(/\/api\/workflows\/callbacks\//)).toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: /Reveal URL/ })); + + expect(await screen.findByDisplayValue(`${window.location.origin}/api/workflows/callbacks/0123456789abcdef0123456789abcdef`)).toBeTruthy(); + expect(reveal.mutateAsync).toHaveBeenCalledWith("wait-cb"); + }); + + it("drops a revealed URL when the pending callback wait changes — the old URL resumes nothing", async () => { + // Parallel callback branches, or a loop that re-parks: the run stays Suspended while the poll moves to another wait. + permissions.held = new Set(["runs.control"]); + reveal.mutateAsync.mockClear(); + reveal.reset.mockClear(); + reveal.mutateAsync.mockResolvedValueOnce({ waitId: "wait-cb", path: "/api/workflows/callbacks/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" }); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: /Reveal URL/ })); + expect(await screen.findByDisplayValue(/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$/)).toBeTruthy(); + + rerender(); + + expect(screen.queryByDisplayValue(/\/api\/workflows\/callbacks\//)).toBeNull(); + expect(screen.getByRole("button", { name: /Reveal URL/ })).toBeTruthy(); + expect(reveal.mutateAsync).toHaveBeenCalledTimes(1); + }); + + it("offers a viewer no reveal, and says who can", () => { + permissions.held = new Set(); + reveal.mutateAsync.mockClear(); + render(); + + expect(screen.queryByRole("button", { name: /Reveal URL/ })).toBeNull(); + expect(screen.getByText(/Only members who can control runs can reveal it/)).toBeTruthy(); + expect(reveal.mutateAsync).not.toHaveBeenCalled(); + }); + + it("shows the URL an API from before the reveal still projects — this page served first in a deploy, or the API rolled back alone", () => { + // That API has no reveal route and hands every member the token itself, as it always did; asking it would answer 404. + permissions.held = new Set(["runs.control"]); + reveal.mutateAsync.mockClear(); + render(); + + expect(screen.getByDisplayValue(`${window.location.origin}/api/workflows/callbacks/0123456789abcdef0123456789abcdef`)).toBeTruthy(); + expect(reveal.mutateAsync).not.toHaveBeenCalled(); }); it("embeds the live child run for a Subworkflow wait, with its approval operable inline", () => { @@ -96,11 +153,11 @@ describe("SuspendedPanel", () => { id: "child-1", workflowId: "w", workflowVersion: 1, sourceType: "workflow.child", normalizedPayload: {}, status: "Suspended", error: null, startedAt: null, completedAt: null, nodes: [{ nodeId: "review", iterationKey: "", status: "Suspended", inputs: {}, outputs: {}, error: null, startedAt: null, completedAt: null }], - pendingWait: { nodeId: "review", kind: "Approval", token: "ctok", payload: { prompt: "Approve the review?" } }, + pendingWait: { id: "wait-review", nodeId: "review", kind: "Approval", token: "ctok", payload: { prompt: "Approve the review?" } }, }, }); - render(); + render(); // The embedded child's bounded pending action is visible… expect(screen.getByText("Approve the review?")).toBeTruthy(); // the child's approval prompt @@ -112,7 +169,7 @@ describe("SuspendedPanel", () => { it("stops embedding once nesting is too deep — shows the child run id instead of fetching", () => { useWorkflowRunMock.mockClear(); - render(); + render(); expect(screen.getByText("deep-child")).toBeTruthy(); expect(useWorkflowRunMock).not.toHaveBeenCalled(); diff --git a/frontend/src/hooks/use-team-management.ts b/frontend/src/hooks/use-team-management.ts index 96902a01c..a37dd3989 100644 --- a/frontend/src/hooks/use-team-management.ts +++ b/frontend/src/hooks/use-team-management.ts @@ -35,6 +35,8 @@ export const TeamPermissions = { // read-only one. The UI still gates its writes on it: a control that can only answer 403 is worse // than no control. StorageManage: "storage.manage", + // Resuming, continuing and cancelling a run and re-issuing its waits — and revealing a callback URL, which resumes it without a session. + RunsControl: "runs.control", } as const; /** Pending invitations. Only fetched when the caller may manage members — the endpoint refuses otherwise. */ diff --git a/frontend/src/hooks/use-workflows.ts b/frontend/src/hooks/use-workflows.ts index b6bba6491..e09022095 100644 --- a/frontend/src/hooks/use-workflows.ts +++ b/frontend/src/hooks/use-workflows.ts @@ -622,6 +622,11 @@ export function useCancelRun(runId: string) { }); } +/** Reveal a pending Callback wait's URL. Not cached: it is a credential, fetched only when someone asks to see it. */ +export function useRevealCallbackUrl(runId: string) { + return useMutation({ mutationFn: (waitId: string) => workflowsApi.revealCallbackUrl(runId, waitId), gcTime: 0 }); +} + /** Approve / reject a run parked on a flow.wait_approval, then resume it. */ export function useResumeRun(runId: string) { const qc = useQueryClient(); diff --git a/frontend/src/static-hosting.test.ts b/frontend/src/static-hosting.test.ts index cfe783151..de4aed4d7 100644 --- a/frontend/src/static-hosting.test.ts +++ b/frontend/src/static-hosting.test.ts @@ -123,6 +123,83 @@ describe("SPA response headers (nginx.conf)", () => { }); }); +/** The `map { … }` block for `name`: its regex entries (nginx PCRE, which these named groups share with JS) and its default. */ +function nginxMap(name: string): { entries: Array<{ pattern: RegExp; value: string }>; fallback: string } { + const block = new RegExp(`^map\\s+\\$\\w+\\s+\\$${name}\\s*\\{([\\s\\S]*?)^\\}`, "m").exec(nginxConf); + + expect(block, `nginx.conf has no map defining $${name}`).not.toBeNull(); + const entries = Array.from(block![1].matchAll(/^\s*"~(\*?)(.*)"\s+"(.*)";\s*$/gm), ([, insensitive, source, value]) => ({ pattern: new RegExp(source, insensitive ? "i" : ""), value })); + const fallback = /^\s*default\s+(\S+);/m.exec(block![1]); + + expect(fallback, `the $${name} map has no default`).not.toBeNull(); + return { entries, fallback: fallback![1] }; +} + +/** What nginx writes for `$name` when its map's source is `input`: the first matching regex's value with its named captures filled in, else the default. */ +function mapped(name: string, input: string): string { + const { entries, fallback } = nginxMap(name); + const hit = entries.map(entry => ({ entry, match: entry.pattern.exec(input) })).find(({ match }) => match !== null); + + if (!hit) return fallback.startsWith("$") ? input : fallback; + return hit.entry.value.replace(/\$\{(\w+)\}/g, (_, group: string) => hit.match!.groups?.[group] ?? ""); +} + +/** The access log format the SPA's server writes with. */ +function accessLogFormat(): string { + const directive = /^\s{4}access_log\s+\S+\s+(\w+);/m.exec(nginxConf); + + expect(directive, "the server block names no access_log format — the image's default `main` writes $request and the Referer verbatim").not.toBeNull(); + const format = new RegExp(`^log_format\\s+${directive![1]}\\s+([\\s\\S]*?);\\s*$`, "m").exec(nginxConf); + + expect(format, `nginx.conf does not define log_format ${directive![1]}`).not.toBeNull(); + return format![1]; +} + +describe("SPA access log (nginx.conf)", () => { + const canary = "CANARY-nginx-9f2c41d7e8"; + + it("writes the request line and the Referer only through the maps that take a bearer token out", () => { + const format = accessLogFormat(); + + expect(format).toContain("$codespace_logged_uri"); + expect(format).toContain("$codespace_logged_referer"); + expect(format).not.toMatch(/\$request(?![_a-z])/); + expect(format).not.toContain("$request_uri"); + expect(format).not.toContain("$http_referer"); + }); + + it.each([ + [`/invite/${canary}`, "/invite/{token}"], + [`/reset-password/${canary}?from=mail`, "/reset-password/{token}?from=mail"], + [`/api/invitations/${canary}/accept`, "/api/invitations/{token}/accept"], + [`/api/auth/reset-password/${canary}`, "/api/auth/reset-password/{token}"], + [`/api/workflows/callbacks/${canary}`, "/api/workflows/callbacks/{token}"], + [`/INVITE/${canary}`, "/INVITE/{token}"], + ["/teams/demo/runs/7", "/teams/demo/runs/7"], + ["/assets/index-3f2a.js", "/assets/index-3f2a.js"], + ])("logs the request for %s as %s", (uri, logged) => { + expect(mapped("codespace_logged_uri", uri)).toBe(logged); + }); + + it.each([ + [`https://app.example.test/invite/${canary}`, "https://app.example.test/invite/{token}"], + [`http://127.0.0.1:8080/reset-password/${canary}`, "http://127.0.0.1:8080/reset-password/{token}"], + ["https://app.example.test/teams/demo/runs", "https://app.example.test/teams/demo/runs"], + ["", ""], + ])("logs the Referer %s as %s — a page at a token link sends its URL with every asset it loads", (referer, logged) => { + expect(mapped("codespace_logged_referer", referer)).toBe(logged); + }); + + it("covers every SPA route whose path carries a $token", () => { + // A new token route added to src/routes must be added to the maps too, or its token reaches the log. + const routes = Object.keys(import.meta.glob("./routes/*.tsx")).filter(path => !path.includes(".test.") && path.includes("$token")); + const prefixes = routes.map(path => "/" + path.replace("./routes/", "").replace(/\.tsx$/, "").split(".$token")[0].split(".").join("/") + "/"); + + expect(prefixes).toEqual(expect.arrayContaining(["/invite/", "/reset-password/"])); + for (const prefix of prefixes) expect(mapped("codespace_logged_uri", `${prefix}${canary}`), prefix).toBe(`${prefix}{token}`); + }); +}); + describe("the SPA's runtime env script", () => { it("is loaded from /env.js — index.html carries no inline script a strict script-src would block", () => { const scripts = indexHtml.match(/]*>/g) ?? [];