diff --git a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs index cda2402d9..9bd59723f 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs @@ -11,6 +11,7 @@ using CodeSpace.Core.Services.Agents.Cost; using CodeSpace.Core.Services.Agents.Mcp; using CodeSpace.Core.Services.Agents.Publish; +using CodeSpace.Core.Services.Redaction; using CodeSpace.Core.Services.Review; using CodeSpace.Core.Services.Supervisor; using CodeSpace.Core.Services.Workflows.Artifacts; @@ -4229,13 +4230,8 @@ private sealed record ModelCredentialProjection( public ResolvedModelCredential? Credential { get; init; } } - /// - /// The shortest value usable as a redaction needle. Below this a value is a fragment, not an identifier — - /// striking it would shred unrelated text (an injected 1 would hit every line carrying a digit), so it is - /// left readable. The SAME threshold the real-model artifact redactor applies (MIN_NEEDLE_LENGTH in - /// .github/scripts/collect-real-model-verdicts.sh); no real api key or access token is shorter. - /// - internal const int MinimumNeedleLength = 8; + /// The shortest value usable as a redaction needle — the shared , so the run's redactor and every other echo scrub agree on what counts as a fragment. + internal const int MinimumNeedleLength = CredentialNeedles.MinimumLength; /// /// The most needles will hand back. is @@ -4254,28 +4250,8 @@ private sealed record ModelCredentialProjection( /// internal const int MaximumNeedles = 64; - /// - /// Name fragments that mark an injected value a SECRET — applied to an entry's - /// variable name and to a base URL's query-parameter name alike. Both are opaque name/value string pairs with no - /// per-entry secret flag for the executor to read, so the name is the only signal available — and it is the signal - /// that separates the token from the values beside it that must stay READABLE: the gateway base URL, the model - /// tier pins, an api-version stamp. Those are what an operator reads the error FOR, and masking a low-entropy - /// one would shred every unrelated line that happens to carry it (an api-version date also matches every - /// timestamp of that day). - /// - /// Substring matching over a deliberately WIDE list: a marker earns its place by naming a carrier that - /// authenticates something (PASSPHRASE on a signing key, PWD on MYSQL_PWD, DSN and - /// CONNECTION_STRING on a database URL with its password inline, COOKIE on a session, WEBHOOK - /// on a URL whose path IS the capability). The known cost is a false-positive class — KEY_ID, - /// TOKEN_LIMIT, an injected PWD — whose values are masked though they are not secret. That is the - /// side to err on: a masked path is a garbled line an operator can work around, an unmasked token is a leak that - /// the append-only log can never take back. - /// - private static readonly string[] SecretNameMarkers = - ["KEY", "TOKEN", "SECRET", "PASSWORD", "PASSWD", "PASSPHRASE", "PWD", "CREDENTIAL", "AUTH", "COOKIE", "DSN", "CONNECTION_STRING", "CONNSTR", "PRIVATE", "WEBHOOK"]; - - /// Whether a name — an env variable's or a URL query parameter's — marks its value a secret. - private static bool MarksASecret(string name) => SecretNameMarkers.Any(marker => name.Contains(marker, StringComparison.OrdinalIgnoreCase)); + /// Whether a name — an env variable's or a URL query parameter's — marks its value a secret (). + private static bool MarksASecret(string name) => CredentialNeedles.MarksASecret(name); /// /// The run's redactor, built from EVERY secret this launch injects into the child process — not the model key @@ -4359,39 +4335,8 @@ private static SecretRedactor WithMcpRunToken(SecretRedactor redactor, string? m internal static SecretRedactor WithModelBrokerRunToken(SecretRedactor redactor, string? brokerRunToken) => brokerRunToken is { Length: >= MinimumNeedleLength } token ? redactor.With([token]) : redactor; - /// - /// The credential parts embedded IN a base URL — every userinfo segment, and each query value whose parameter - /// name marks it a secret — in both the raw and the percent-decoded spelling, - /// since a CLI may echo either. Userinfo is credential material by construction, so all of it is a needle; a - /// query string is not, so it is filtered by name. The URL itself is never a needle: its host is the fact that - /// says WHICH endpoint answered, which is the point of surfacing the error at all. - /// - /// ACCEPTED RESIDUAL: a gateway that carries its key in the URL PATH (https://gw.example/v1/<key>/chat) - /// is not covered. A path segment has no name to read, so the only rules available are "mask every segment" — - /// which strikes the route an operator reads the error for, and every /v1/ beside it — or a shape guess, - /// which is a fresh leak the moment a gateway picks a shape it doesn't match. Userinfo and a marked query - /// parameter are self-declaring; a path segment is not, so it stays out until a credential can say so itself. - /// - private static IEnumerable UrlEmbeddedSecrets(string? baseUrl) - { - if (!Uri.TryCreate(baseUrl, UriKind.Absolute, out var parsed)) yield break; - - foreach (var part in parsed.UserInfo.Split(':', StringSplitOptions.RemoveEmptyEntries)) - { - yield return part; - yield return Uri.UnescapeDataString(part); - } - - foreach (var pair in parsed.Query.TrimStart('?').Split('&', StringSplitOptions.RemoveEmptyEntries)) - { - var separator = pair.IndexOf('='); - - if (separator < 0 || !MarksASecret(pair[..separator])) continue; - - yield return pair[(separator + 1)..]; - yield return Uri.UnescapeDataString(pair[(separator + 1)..]); - } - } + /// The credential parts embedded IN a base URL — userinfo and secret-named query values (). + private static IEnumerable UrlEmbeddedSecrets(string? baseUrl) => CredentialNeedles.FromUrl(baseUrl); /// Re-persist the run's stored task with its RESOLVED model filled, so the live projection shows what an "auto" run actually dispatches from the moment it starts (mirrors the harness-reconciliation write). The task is the ORIGINAL (no injected secret env) with only set — but it is the OPENED task, so it is re-sealed exactly as the store first persisted it: written back raw, the workflow secrets it embeds would land in task_jsonb again. private Task PersistResolvedModelAsync(AgentRunOwnerToken owner, AgentTask taskWithModel, CancellationToken cancellationToken) => PersistRuntimeIdentityAsync(owner, null, AgentTaskSecrets.Seal(taskWithModel, _sealer), cancellationToken); diff --git a/backend/src/CodeSpace.Core/Services/Agents/ModelCredentials/ModelCredentialService.cs b/backend/src/CodeSpace.Core/Services/Agents/ModelCredentials/ModelCredentialService.cs index 398298247..a53306b0e 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/ModelCredentials/ModelCredentialService.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/ModelCredentials/ModelCredentialService.cs @@ -4,10 +4,13 @@ using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Identity; +using CodeSpace.Core.Services.Redaction; using CodeSpace.Messages.Agents; using CodeSpace.Messages.Dtos.ModelCredentials; using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; namespace CodeSpace.Core.Services.Agents.ModelCredentials; @@ -15,15 +18,19 @@ public sealed class ModelCredentialService : IModelCredentialService, IScopedDep { private readonly CodeSpaceDbContext _db; private readonly ICurrentTeam _currentTeam; + private readonly ICurrentUser _currentUser; private readonly IPayloadEncryptor _encryptor; private readonly IModelReflector _reflector; + private readonly ILogger _logger; - public ModelCredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, IPayloadEncryptor encryptor, IModelReflector reflector) + public ModelCredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, IModelReflector reflector, ILogger logger) { _db = db; _currentTeam = currentTeam; + _currentUser = currentUser; _encryptor = encryptor; _reflector = reflector; + _logger = logger; } public async Task> ListAsync(string? provider, CancellationToken cancellationToken) @@ -64,9 +71,13 @@ public async Task AddAsync(string provider, string displayName, string? ap public async Task UpdateAsync(Guid id, string displayName, string? apiKey, string? baseUrl, CancellationToken cancellationToken) { var credential = await LoadActiveAsync(id, cancellationToken).ConfigureAwait(false); + var nextBaseUrl = SubmittedBaseUrl(credential.BaseUrl, baseUrl); + + EnsureKeyFollowsEndpoint(credential, nextBaseUrl, apiKey); + LogEndpointMove(credential, nextBaseUrl); credential.DisplayName = displayName; - credential.BaseUrl = NullIfBlank(baseUrl); + credential.BaseUrl = nextBaseUrl; // Write-only secret: a blank key keeps the existing one; a value rotates it. if (!string.IsNullOrWhiteSpace(apiKey)) credential.EncryptedApiKey = _encryptor.Encrypt(apiKey); @@ -76,6 +87,36 @@ public async Task UpdateAsync(Guid id, string displayName, string? apiKey, return credential.Id; } + /// + /// The base URL an update stores. The list shows it MASKED (), so an edit form + /// that sends back exactly what it was shown means "unchanged" — the stored URL, with its real embedded credentials, + /// is kept; a masked placeholder left in any other submitted URL is refused (). + /// + internal static string? SubmittedBaseUrl(string? stored, string? submitted) => CredentialUrl.KeepStoredIfMasked(stored, NullIfBlank(submitted)); + + /// + /// Whether an update must carry the key again: it moves the endpoint (scheme, host or port — where a key is sent), + /// a key is stored, and none is supplied. Without this, anyone allowed to edit the credential could repoint it at a + /// host they control and have the next reflection, probe or run present the write-only key there. A path-only change, + /// a keyless credential, or an update that supplies the key moves nothing it shouldn't. + /// + internal static bool KeyMustBeReentered(bool hasStoredKey, string? storedBaseUrl, string? nextBaseUrl, string? apiKey) => + hasStoredKey && string.IsNullOrWhiteSpace(apiKey) && !CredentialUrl.SameEndpoint(storedBaseUrl, nextBaseUrl); + + private static void EnsureKeyFollowsEndpoint(ModelCredential credential, string? nextBaseUrl, string? apiKey) + { + if (KeyMustBeReentered(!string.IsNullOrEmpty(credential.EncryptedApiKey), credential.BaseUrl, nextBaseUrl, apiKey)) + throw new ModelEndpointKeyRequiredException(credential.Id); + } + + /// One audit line per endpoint move, naming only scheme://host:port on each side () — never the embedded credentials, path or query. + private void LogEndpointMove(ModelCredential credential, string? nextBaseUrl) + { + if (CredentialUrl.SameEndpoint(credential.BaseUrl, nextBaseUrl)) return; + + _logger.LogInformation("Model credential {CredentialId} of team {TeamId} moved its endpoint from {PreviousEndpoint} to {Endpoint}, by user {UserId}", credential.Id, credential.TeamId, CredentialUrl.Endpoint(credential.BaseUrl), CredentialUrl.Endpoint(nextBaseUrl), _currentUser.Id); + } + public async Task RevokeAsync(Guid id, CancellationToken cancellationToken) { var teamId = _currentTeam.Id!.Value; @@ -324,7 +365,9 @@ private ModelCredentialSummary ToSummary(ModelCredential c) // null (a non-blank secret is never stored). Surface it so the UI prompts re-entry instead of showing a // dead key as the benign "no key". KeyUnreadable = !string.IsNullOrEmpty(c.EncryptedApiKey) && keyHint is null, - BaseUrl = c.BaseUrl, + // Masked: userinfo and secret-named query values are credentials, and the list reaches every member. + // Update reads this exact string sent back as "keep the stored URL". + BaseUrl = CredentialUrl.Masked(c.BaseUrl), Status = c.Status, CreatedDate = c.CreatedDate, }; diff --git a/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs b/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs index 04bbc05f8..c0eabf03b 100644 --- a/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs +++ b/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs @@ -4,13 +4,16 @@ using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Identity; using CodeSpace.Core.Middlewares.Transactional; +using CodeSpace.Core.Services.Redaction; using CodeSpace.Core.Services.RepositoryBinding; using CodeSpace.Core.Services.Webhooks.Scope; using CodeSpace.Messages.Commands.ProviderInstances; using CodeSpace.Messages.Dtos.ProviderInstances; using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; using CodeSpace.Messages.Queries.ProviderInstances; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; namespace CodeSpace.Core.Services.ProviderInstances; @@ -18,21 +21,25 @@ public sealed class ProviderInstanceService : IProviderInstanceService, IScopedD { private readonly CodeSpaceDbContext _db; private readonly ICurrentTeam _currentTeam; + private readonly ICurrentUser _currentUser; private readonly IPayloadEncryptor _encryptor; private readonly IRepositoryBindingService _binding; private readonly IWebhookScopeTransitionService _webhookScope; private readonly IPostCommitActions _postCommit; private readonly TimeProvider _clock; + private readonly ILogger _logger; - public ProviderInstanceService(CodeSpaceDbContext db, ICurrentTeam currentTeam, IPayloadEncryptor encryptor, IRepositoryBindingService binding, IWebhookScopeTransitionService webhookScope, IPostCommitActions postCommit, TimeProvider clock) + public ProviderInstanceService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, IRepositoryBindingService binding, IWebhookScopeTransitionService webhookScope, IPostCommitActions postCommit, TimeProvider clock, ILogger logger) { _db = db; _currentTeam = currentTeam; + _currentUser = currentUser; _encryptor = encryptor; _binding = binding; _webhookScope = webhookScope; _postCommit = postCommit; _clock = clock; + _logger = logger; } public async Task> ListAsync(CancellationToken cancellationToken) @@ -48,9 +55,11 @@ public async Task> ListAsync(Cancellation TeamId = p.TeamId, Provider = p.Provider, DisplayName = p.DisplayName, - BaseUrl = p.BaseUrl, - ApiUrl = p.ApiUrl, - WebUrl = p.WebUrl, + // Masked: userinfo and secret-named query values are credentials, and this list reaches every member. + // Update reads each exact string sent back as "keep the stored URL". + BaseUrl = CredentialUrl.Masked(p.BaseUrl)!, + ApiUrl = CredentialUrl.Masked(p.ApiUrl), + WebUrl = CredentialUrl.Masked(p.WebUrl), CreatedDate = p.CreatedDate, OauthEnabled = p.OauthClientId != null && p.OauthClientSecretEnc != null }) @@ -112,24 +121,94 @@ public async Task UpdateAsync(UpdateProviderInstanceCommand request, Cancellatio var teamId = _currentTeam.Id!.Value; var instance = await LoadOwnedInstanceAsync(request.ProviderInstanceId, teamId, cancellationToken).ConfigureAwait(false); + var edits = WithStoredUrlsKept(instance, request); // Compute the post-edit (base_url, client_id) tuple — needed for the uniqueness // check. Only run the check when at least one of those keys is actually changing, // because the no-op case would self-collide on the row's own values. - var newBaseUrl = request.BaseUrl != null ? NormaliseBaseUrl(request.BaseUrl) : instance.BaseUrl; - var newClientId = request.OauthClientId != null ? NormaliseClientId(request.OauthClientId) : instance.OauthClientId; + var newBaseUrl = NextBaseUrl(instance, edits); + var newClientId = edits.OauthClientId != null ? NormaliseClientId(edits.OauthClientId) : instance.OauthClientId; var baseUrlChanged = !string.Equals(newBaseUrl, instance.BaseUrl, StringComparison.Ordinal); var clientIdChanged = !string.Equals(newClientId, instance.OauthClientId, StringComparison.Ordinal); if (baseUrlChanged || clientIdChanged) await EnsureUniquenessKeyAvailableAsync(instance, newBaseUrl, newClientId, cancellationToken).ConfigureAwait(false); + await EnsureSecretsStayWithTheirEndpointAsync(instance, edits, cancellationToken).ConfigureAwait(false); + LogEndpointMove(instance, edits); + var previousScope = instance.WebhookScope; - ApplyEdits(instance, request); + ApplyEdits(instance, edits); await ApplyWebhookScopeChangeAsync(instance, previousScope, cancellationToken).ConfigureAwait(false); } + /// + /// The edit with each URL the list shows MASKED () read back as "unchanged" when it + /// is sent back exactly as shown, so an edit form or API client that PATCHes back what it GETs keeps the stored URL + /// and its real embedded credentials; a placeholder left in an otherwise edited URL is refused. Resolved before the + /// uniqueness check, so a masked echo of the row's own URL never reads as a different one. + /// + private static UpdateProviderInstanceCommand WithStoredUrlsKept(ProviderInstance instance, UpdateProviderInstanceCommand request) => request with + { + BaseUrl = request.BaseUrl == null ? null : CredentialUrl.KeepStoredIfMasked(instance.BaseUrl, NormaliseBaseUrl(request.BaseUrl)), + ApiUrl = request.ApiUrl == null ? null : CredentialUrl.KeepStoredIfMasked(instance.ApiUrl, request.ApiUrl), + WebUrl = request.WebUrl == null ? null : CredentialUrl.KeepStoredIfMasked(instance.WebUrl, request.WebUrl) + }; + + /// + /// Whether an edit moves where this connection presents its secrets: the API every provider call goes to + /// (ApiUrl, or the base URL when unset) or the base URL the OAuth token, authorize and revoke endpoints are + /// derived from. A move is a change of scheme, host or port (); a path, a + /// trailing slash or a credential slot is not. + /// + internal static bool EndpointMoves(string storedBaseUrl, string? storedApiUrl, string nextBaseUrl, string? nextApiUrl) => + !CredentialUrl.SameEndpoint(storedBaseUrl, nextBaseUrl) || !CredentialUrl.SameEndpoint(ApiOf(storedBaseUrl, storedApiUrl), ApiOf(nextBaseUrl, nextApiUrl)); + + /// + /// Refuse a repoint the connection's secrets would follow. Its connected tokens are write-only and belong to the + /// members who connected them, so an editor who may change the URL could otherwise point it at a host they control + /// and have the next repository read, git node, webhook registration or OAuth refresh present every one of them + /// there — they are disconnected first instead. The stored OAuth client secret is the editor's to re-enter, as a + /// model key is. A connection with nothing bound moves freely. + /// + private async Task EnsureSecretsStayWithTheirEndpointAsync(ProviderInstance instance, UpdateProviderInstanceCommand edits, CancellationToken cancellationToken) + { + if (!EndpointMoves(instance.BaseUrl, instance.ApiUrl, NextBaseUrl(instance, edits), NextApiUrl(instance, edits))) return; + + var bound = await CountPresentableCredentialsAsync(instance.Id, cancellationToken).ConfigureAwait(false); + + if (bound > 0) + throw new ProviderEndpointCredentialsBoundException(instance.Id, bound); + + if (instance.OauthClientSecretEnc != null && string.IsNullOrEmpty(edits.OauthClientSecret)) + throw new ProviderEndpointSecretRequiredException(instance.Id); + } + + /// Credentials that would still be presented: connected through this instance and not disconnected (a disconnect revokes the row and blanks its token). + private Task CountPresentableCredentialsAsync(Guid providerInstanceId, CancellationToken cancellationToken) => + _db.Credential.AsNoTracking().CountAsync(c => c.ProviderInstanceId == providerInstanceId && c.DeletedDate == null && c.Status != CredentialStatus.Revoked, cancellationToken); + + /// One audit line per endpoint move, naming only scheme://host:port on each side () — never embedded credentials, path or query. + private void LogEndpointMove(ProviderInstance instance, UpdateProviderInstanceCommand edits) + { + var nextBaseUrl = NextBaseUrl(instance, edits); + var nextApiUrl = NextApiUrl(instance, edits); + + if (!EndpointMoves(instance.BaseUrl, instance.ApiUrl, nextBaseUrl, nextApiUrl)) return; + + _logger.LogInformation("Provider instance {ProviderInstanceId} of team {TeamId} moved its endpoint from {PreviousEndpoint} (API {PreviousApiEndpoint}) to {Endpoint} (API {ApiEndpoint}), by user {UserId}", instance.Id, instance.TeamId, CredentialUrl.Endpoint(instance.BaseUrl), CredentialUrl.Endpoint(ApiOf(instance.BaseUrl, instance.ApiUrl)), CredentialUrl.Endpoint(nextBaseUrl), CredentialUrl.Endpoint(ApiOf(nextBaseUrl, nextApiUrl)), _currentUser.Id); + } + + private static string NextBaseUrl(ProviderInstance instance, UpdateProviderInstanceCommand edits) => edits.BaseUrl != null ? NormaliseBaseUrl(edits.BaseUrl) : instance.BaseUrl; + + private static string? NextApiUrl(ProviderInstance instance, UpdateProviderInstanceCommand edits) => edits.ApiUrl == null ? instance.ApiUrl : NullIfBlank(edits.ApiUrl); + + /// Where provider calls go: the API URL when one is set, the base URL otherwise — the fallback every provider reads. + private static string ApiOf(string baseUrl, string? apiUrl) => string.IsNullOrWhiteSpace(apiUrl) ? baseUrl : apiUrl; + + private static string? NullIfBlank(string value) => string.IsNullOrWhiteSpace(value) ? null : value; + /// /// Deferred to after the edit commits, for the reason a bind defers its registration: the /// transition reads the new scope off the row and dispatches registrations against it, and a diff --git a/backend/src/CodeSpace.Core/Services/Providers/Auth/PresentedProviderCredentials.cs b/backend/src/CodeSpace.Core/Services/Providers/Auth/PresentedProviderCredentials.cs new file mode 100644 index 000000000..411e0a8d9 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Providers/Auth/PresentedProviderCredentials.cs @@ -0,0 +1,32 @@ +using System.Collections.Concurrent; +using System.Runtime.CompilerServices; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Agents; +using CodeSpace.Core.Services.Redaction; + +namespace CodeSpace.Core.Services.Providers.Auth; + +/// +/// The tokens a provider instance has been handed to present, so the SDK-error translation in +/// can strike an upstream's echo of them. A provider call resolves its +/// token through and then runs under the resilience layer with the SAME +/// object (context.Instance); the resilience layer is a singleton that never sees +/// the credential, so the token travels beside that object instead. +/// +/// Keyed by the entity OBJECT, held weakly: the needles live exactly as long as the scope that loaded the +/// instance, never in a process-wide list, and a second scope's instance object starts empty. A scope that resolves two +/// credentials of one instance strikes both — over-striking an error line is the safe side. +/// +internal static class PresentedProviderCredentials +{ + private static readonly ConditionalWeakTable> Presented = new(); + + /// Remember that was handed to present (ignored when too short to be a needle). + public static void Remember(ProviderInstance instance, string? token) + { + foreach (var needle in CredentialNeedles.Usable([token])) Presented.GetOrCreateValue(instance).TryAdd(needle, 0); + } + + /// A redactor over every token was handed; when it was handed none. + public static SecretRedactor RedactorFor(ProviderInstance instance) => Presented.TryGetValue(instance, out var needles) ? CredentialNeedles.Redactor(needles.Keys) : SecretRedactor.None; +} diff --git a/backend/src/CodeSpace.Core/Services/Providers/Auth/ProviderAuthResolver.cs b/backend/src/CodeSpace.Core/Services/Providers/Auth/ProviderAuthResolver.cs index 005e04ca5..fe036f026 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Auth/ProviderAuthResolver.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Auth/ProviderAuthResolver.cs @@ -15,6 +15,12 @@ public async Task ResolveAsync(ProviderContext context, Cancellati if (!_strategies.TryGetValue(key, out var strategy)) throw new NotSupportedException($"No auth strategy registered for ({key.Provider}, {key.AuthType})"); - return await strategy.ResolveAsync(context, cancellationToken).ConfigureAwait(false); + var auth = await strategy.ResolveAsync(context, cancellationToken).ConfigureAwait(false); + + // Every token handed out here is one the provider may echo back in an error — remember it beside the instance + // so the resilience layer's SDK-error translation can strike it before a caller is shown the failure. + PresentedProviderCredentials.Remember(context.Instance, auth.Token); + + return auth; } } diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.ConnectionWebhooks.cs b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.ConnectionWebhooks.cs index 88ddc3d0c..4988edca3 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.ConnectionWebhooks.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.ConnectionWebhooks.cs @@ -33,7 +33,7 @@ public sealed partial class GitHubRepositoryProvider : IConnectionWebhookRegistr { // Failing to LIST fails the registration exactly as surely as failing to create, and for // the same reasons — so the operator gets the same evidence from either step. - throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureOrgHookRequest("GET", baseAddress, ownerPath, token, null)), ex); + throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureOrgHookRequest("GET", baseAddress, ownerPath, token, null), token), ex); } } @@ -59,7 +59,7 @@ public async Task RegisterConnectionWebhookAsync(ProviderContext catch (Exception ex) { var body = JsonSerializer.Serialize(new { name = "web", config, events = GitHubHookEvents.All, active = true }); - throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureOrgHookRequest("POST", baseAddress, ownerPath, token, body)), ex); + throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureOrgHookRequest("POST", baseAddress, ownerPath, token, body), token), ex); } } diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs index d7cf0c17c..40b2cad41 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs @@ -7,6 +7,7 @@ using CodeSpace.Core.Services.Providers.Markdown; using CodeSpace.Core.Services.Providers.Resilience; using CodeSpace.Core.Services.Providers.Source; +using CodeSpace.Core.Services.Redaction; using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Events; using CodeSpace.Messages.Exceptions; @@ -848,7 +849,7 @@ internal static RepositoryRole MapPermissions(bool admin, bool maintain, bool pu // Failing to LIST hooks fails the registration exactly as surely as failing to create // one, and for the same reasons (revoked token, no permission, instance down) — so the // operator gets the same evidence from either step. - throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureHookRequest("GET", baseAddress, repositoryId, token, null)), ex); + throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureHookRequest("GET", baseAddress, repositoryId, token, null), token), ex); } } @@ -892,7 +893,7 @@ public async Task RegisterWebhookAsync(ProviderContext context, R catch (Exception ex) { var body = JsonSerializer.Serialize(new { name = "web", config, events = GitHubHookEvents.All, active = true }); - throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureHookRequest("POST", baseAddress, repositoryId, token, body)), ex); + throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureHookRequest("POST", baseAddress, repositoryId, token, body), token), ex); } } @@ -959,15 +960,19 @@ private static CapturedProviderRequest CaptureHookRequest(string method, Uri bas /// is rarely the one we caught — the resilience layer translates it and the scope mapper replaces /// it — so we look down the chain for the shape that still knows what GitHub said. Finding /// nothing is itself informative: no status means the call never got an answer. + /// + /// The body is Octokit's raw copy of GitHub's answer — the resilience translation strikes only the message — + /// so the the request presented is struck from it here: the attempt row is readable by any + /// member of the team, and a GitHub (or a proxy) that quotes the credential it rejected would hand it to all of them. /// - private static ProviderCallDiagnostic DescribeHookFailure(Exception exception, CapturedProviderRequest request) + private static ProviderCallDiagnostic DescribeHookFailure(Exception exception, CapturedProviderRequest request, string token) { var api = ProviderCallCapture.FindInChain(exception); return new ProviderCallDiagnostic { StatusCode = api == null ? null : (int)api.StatusCode, - ResponseBody = ProviderCallCapture.Clamp(api?.HttpResponse?.Body as string), + ResponseBody = ProviderCallCapture.Clamp(CredentialNeedles.Strike(api?.HttpResponse?.Body as string, [token])), Request = request }; } diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.ConnectionWebhooks.cs b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.ConnectionWebhooks.cs index d2a0f50fa..808fcb232 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.ConnectionWebhooks.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.ConnectionWebhooks.cs @@ -4,6 +4,7 @@ using CodeSpace.Core.Services.Providers.Capabilities; using CodeSpace.Core.Services.Providers.Diagnostics; using CodeSpace.Core.Services.Providers.Resilience; +using CodeSpace.Core.Services.Redaction; using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Enums; using CodeSpace.Messages.Exceptions; @@ -132,6 +133,11 @@ private static async Task> ListGroupHooksAsync(string url, private static int? NextPageAfter(int current, string? nextPage) => int.TryParse(nextPage, out var next) && next > current ? next : null; + /// + /// One raw call to the group-hook endpoint. A refusal's body is the evidence + /// persists to the attempt row any member can read, so the this request presented is struck + /// from it before anything reads it; an accepted answer is read as sent. + /// private static async Task SendGroupHookRequestAsync(HttpMethod method, string url, string token, string? payload, CancellationToken cancellationToken) { using var request = new HttpRequestMessage(method, url); @@ -144,7 +150,7 @@ private static async Task SendGroupHookRequestAsync(HttpMethod using var response = await _countsHttpClient.SendAsync(request, cancellationToken).ConfigureAwait(false); var body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); - return new GroupHookAnswer(response.StatusCode, body, ReadNextPage(response)); + return new GroupHookAnswer(response.StatusCode, response.IsSuccessStatusCode ? body : CredentialNeedles.Strike(body, [token]), ReadNextPage(response)); } /// GitLab's X-Next-Page: the next page's number on a list, empty on its last page, absent on anything else. diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs index 7c63d49c0..6651f3455 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs @@ -6,6 +6,7 @@ using CodeSpace.Core.Services.Providers.Markdown; using CodeSpace.Core.Services.Providers.Resilience; using CodeSpace.Core.Services.Providers.Source; +using CodeSpace.Core.Services.Redaction; using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Enums; using CodeSpace.Messages.Events; @@ -1048,7 +1049,10 @@ private async Task UnapproveAsync(string host, string token, int projectId, int using var response = await _countsHttpClient.SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); var responseBody = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); - var failure = ClassifyUnapproveResponse((int)response.StatusCode, responseBody); + // The body becomes the failure's detail and scope hint, which reach external_call.failed and the node's error — + // this raw call never passes the resilience translation that strikes an echo, so the token it presented is + // struck here, before classification (which reads only the status and the scope tag). + var failure = ClassifyUnapproveResponse((int)response.StatusCode, CredentialNeedles.Strike(responseBody, [token])); if (failure != null) throw failure; } @@ -1348,7 +1352,7 @@ public async Task GetActorAccessAsync(ProviderContext con // Failing to LIST hooks fails the registration exactly as surely as failing to create // one, and for the same reasons (revoked token, no permission, instance down) — so the // operator gets the same evidence from either step. - throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureHookRequest("GET", host, projectId, token, null)), ex); + throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, CaptureHookRequest("GET", host, projectId, token, null), token), ex); } } @@ -1400,9 +1404,14 @@ public async Task RegisterWebhookAsync(ProviderContext context, R var body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); if (!response.IsSuccessStatusCode) + { + // The refusal's body becomes last_error and the attempt's error, which any member can read: strike the token it may echo. + var refusal = CredentialNeedles.Strike(body, [token]); + throw new ProviderWebhookRegistrationException( - DescribeHookFailure(new HttpRequestException($"GitLab refused the hook with {(int)response.StatusCode}: {body}"), CaptureHookRequest("POST", host, projectId, token, payload)), - new HttpRequestException($"{(int)response.StatusCode}: {body}")); + DescribeHookFailure(new HttpRequestException($"GitLab refused the hook with {(int)response.StatusCode}: {refusal}"), CaptureHookRequest("POST", host, projectId, token, payload), token), + new HttpRequestException($"{(int)response.StatusCode}: {refusal}")); + } return new RemoteWebhook { @@ -1416,7 +1425,7 @@ public async Task RegisterWebhookAsync(ProviderContext context, R catch (Exception ex) { var sent = CaptureHookRequest("POST", host, projectId, token, payload); - throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, sent), ex); + throw new ProviderWebhookRegistrationException(DescribeHookFailure(ex, sent, token), ex); } } @@ -1466,15 +1475,19 @@ private static CapturedProviderRequest CaptureHookRequest(string method, string /// is rarely the one we caught — the resilience layer translates it and the scope mapper replaces /// it — so we look down the chain for the shape that still knows what GitLab said. Finding /// nothing is itself informative: no status means the call never got an answer. + /// + /// The body is the SDK's raw copy of GitLab's answer — the resilience translation strikes only the message — + /// so the the request presented is struck from it here: the attempt row is readable by any + /// member of the team, and a GitLab (or a proxy) that quotes the credential it rejected would hand it to all of them. /// - private static ProviderCallDiagnostic DescribeHookFailure(Exception exception, CapturedProviderRequest request) + private static ProviderCallDiagnostic DescribeHookFailure(Exception exception, CapturedProviderRequest request, string token) { var gitLab = ProviderCallCapture.FindInChain(exception); return new ProviderCallDiagnostic { StatusCode = gitLab == null ? null : (int)gitLab.StatusCode, - ResponseBody = ProviderCallCapture.Clamp(ReadErrorBody(gitLab)), + ResponseBody = ProviderCallCapture.Clamp(CredentialNeedles.Strike(ReadErrorBody(gitLab), [token])), Request = request }; } diff --git a/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs b/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs index b42624792..8bb8bf0de 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs @@ -3,6 +3,7 @@ using System.Threading.RateLimiting; using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Providers.Auth; using CodeSpace.Core.Services.Providers.Errors; using CodeSpace.Messages.Exceptions; using Microsoft.Extensions.Logging; @@ -114,9 +115,13 @@ private void TranslateAndThrowIfScopeIssue(ProviderInstance instance, string ope if (typed == null) return; _logger.LogWarning(ex, "Provider {Provider} returned insufficient_scope for '{Operation}' on instance {InstanceId}; missing scopes: {MissingScopes}", instance.Provider, operationName, instance.Id, string.Join(", ", typed.MissingScopes)); - throw typed; + throw WithClientSafeHint(instance, typed); } + /// The scope failure with its provider hint — returned to the caller in the failure details — made client-safe. Classification already ran on the raw SDK failure, so the scopes it names are unchanged. + private static ProviderInsufficientScopeException WithClientSafeHint(ProviderInstance instance, ProviderInsufficientScopeException typed) => + typed.ProviderHint is null ? typed : new ProviderInsufficientScopeException(typed.ProviderKind, typed.CapabilityName, typed.MissingScopes, typed.GrantedScopes, ClientSafe(instance, typed.ProviderHint), typed.InnerException); + /// /// Re-throws any SDK exception with a known HTTP status code as a typed /// . The check is duck-typed (StatusCode property) @@ -132,10 +137,31 @@ private void TranslateAndThrowIfProviderApi(ProviderInstance instance, string op var status = ExtractStatusCode(ex); if (status == null) return; - _logger.LogWarning(ex, "Provider {Provider} returned HTTP {StatusCode} for '{Operation}' on instance {InstanceId}: {Message}", instance.Provider, status, operationName, instance.Id, ex.Message); - throw new ProviderApiException(instance.Provider, status.Value, operationName, ex.Message, ex); + var providerMessage = ClientSafe(instance, ex.Message); + + _logger.LogWarning(ex, "Provider {Provider} returned HTTP {StatusCode} for '{Operation}' on instance {InstanceId}: {Message}", instance.Provider, status, operationName, instance.Id, providerMessage); + throw new ProviderApiException(instance.Provider, status.Value, operationName, providerMessage, ex); + } + + /// + /// A provider's error text made safe to show the caller: every token the instance was handed to present is struck + /// () — a provider or proxy that echoes the request's PRIVATE-TOKEN / + /// Authorization would otherwise hand it to every Viewer of a repository page and every reader of a git node's + /// error — and the SDK's trailing Original call: <method> <url> is dropped: it names the internal + /// request, and a query-string token rides in it. The SDK exception itself stays the inner exception, for the log. + /// + private static string ClientSafe(ProviderInstance instance, string providerMessage) => PresentedProviderCredentials.RedactorFor(instance).Redact(WithoutOriginalCall(providerMessage)); + + private static string WithoutOriginalCall(string providerMessage) + { + var at = providerMessage.IndexOf(OriginalCallMarker, StringComparison.Ordinal); + + return at < 0 ? providerMessage : providerMessage[..at].TrimEnd(); } + /// How NGitLab (and any SDK copying it) appends the request it made to an error message. + private const string OriginalCallMarker = "Original call:"; + /// /// Charge one attempt. A call's first attempt is charged to the calling agent run's share, then waits its turn for the /// instance's token and may be refused (): nothing has been sent yet. A retry diff --git a/backend/src/CodeSpace.Core/Services/Redaction/CredentialNeedles.cs b/backend/src/CodeSpace.Core/Services/Redaction/CredentialNeedles.cs new file mode 100644 index 000000000..97c0cdd57 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Redaction/CredentialNeedles.cs @@ -0,0 +1,127 @@ +using System.Diagnostics.CodeAnalysis; +using CodeSpace.Core.Services.Agents; +using CodeSpace.Messages.Agents; + +namespace CodeSpace.Core.Services.Redaction; + +/// +/// The values a credential PRESENTS to an upstream — the exact strings an upstream that echoes its input (a 401 body +/// quoting the key it rejected, a proxy that reflects request headers) can hand back into text the platform persists or +/// shows. One rule, shared by every place that has to strike such an echo: the agent run's redactor, the in-process +/// model transport, and the git-provider error translation. +/// +/// Needles are exact values, never shapes. A shape guess (sk-…, glpat-…) is a fresh leak the moment +/// a gateway picks a shape it doesn't match; an exact value is struck wherever it appears, in whatever text. +/// +public static class CredentialNeedles +{ + /// + /// The shortest value usable as a redaction needle. Below this a value is a fragment, not an identifier — + /// striking it would shred unrelated text (an injected 1 would hit every line carrying a digit), so it is + /// left readable. The SAME threshold the real-model artifact redactor applies (MIN_NEEDLE_LENGTH in + /// .github/scripts/collect-real-model-verdicts.sh); no real api key or access token is shorter. + /// + public const int MinimumLength = 8; + + /// + /// Name fragments that mark a value a SECRET — applied to an injected environment variable's name, a URL query + /// parameter's name and a request header's name alike. All three are opaque name/value string pairs with no + /// per-entry secret flag to read, so the name is the only signal available — and it is the signal that separates + /// the token from the values beside it that must stay READABLE: the gateway base URL, the model tier pins, an + /// api-version stamp. Those are what an operator reads the error FOR, and masking a low-entropy one would + /// shred every unrelated line that happens to carry it (an api-version date also matches every timestamp of + /// that day). + /// + /// Substring matching over a deliberately WIDE list: a marker earns its place by naming a carrier that + /// authenticates something (PASSPHRASE on a signing key, PWD on MYSQL_PWD, DSN and + /// CONNECTION_STRING on a database URL with its password inline, COOKIE on a session, WEBHOOK + /// on a URL whose path IS the capability). The known cost is a false-positive class — KEY_ID, + /// TOKEN_LIMIT, an injected PWD — whose values are masked though they are not secret. That is the + /// side to err on: a masked path is a garbled line an operator can work around, an unmasked token is a leak that + /// the append-only log can never take back. + /// + private static readonly string[] SecretNameMarkers = + ["KEY", "TOKEN", "SECRET", "PASSWORD", "PASSWD", "PASSPHRASE", "PWD", "CREDENTIAL", "AUTH", "COOKIE", "DSN", "CONNECTION_STRING", "CONNSTR", "PRIVATE", "WEBHOOK"]; + + /// Whether a name — an env variable's, a URL query parameter's or a header's — marks its value a secret. + public static bool MarksASecret(string name) => SecretNameMarkers.Any(marker => name.Contains(marker, StringComparison.OrdinalIgnoreCase)); + + /// + /// The credential parts embedded IN a URL — every userinfo segment, and each query value whose parameter name + /// marks it a secret — in both the raw and the percent-decoded spelling, since an + /// upstream may echo either. Userinfo is credential material by construction, so all of it is a needle; a query + /// string is not, so it is filtered by name. The URL itself is never a needle: its host is the fact that says WHICH + /// endpoint answered, which is the point of surfacing the error at all. + /// + /// ACCEPTED RESIDUAL: a gateway that carries its key in the URL PATH (https://gw.example/v1/<key>/chat) + /// is not covered. A path segment has no name to read, so the only rules available are "mask every segment" — + /// which strikes the route an operator reads the error for, and every /v1/ beside it — or a shape guess, + /// which is a fresh leak the moment a gateway picks a shape it doesn't match. Userinfo and a marked query + /// parameter are self-declaring; a path segment is not, so it stays out until a credential can say so itself. + /// + public static IEnumerable FromUrl(string? url) + { + if (!Uri.TryCreate(url, UriKind.Absolute, out var parsed)) yield break; + + foreach (var part in parsed.UserInfo.Split(':', StringSplitOptions.RemoveEmptyEntries)) + { + yield return part; + yield return Uri.UnescapeDataString(part); + } + + foreach (var pair in parsed.Query.TrimStart('?').Split('&', StringSplitOptions.RemoveEmptyEntries)) + { + var separator = pair.IndexOf('='); + + if (separator < 0 || !MarksASecret(pair[..separator])) continue; + + yield return pair[(separator + 1)..]; + yield return Uri.UnescapeDataString(pair[(separator + 1)..]); + } + } + + /// Everything a resolved model credential presents: its api key, and the credential parts of its base URL. + public static IEnumerable FromCredential(ResolvedModelCredential? credential) + { + if (credential?.ApiKey is { } apiKey) yield return apiKey; + + foreach (var needle in FromUrl(credential?.BaseUrl)) yield return needle; + } + + /// + /// Everything an outgoing request presents: each header whose name marks it a secret + /// (Authorization, x-api-key, api-key, PRIVATE-TOKEN) and the credential parts of its URL. + /// A scheme credential header value (Bearer sk-…) yields the credential alone: the scheme word is not a + /// secret, and striking the credential is what leaves an echoed Authorization=Bearer *** legible. + /// + public static IEnumerable FromRequest(HttpRequestMessage request) + { + foreach (var header in request.Headers.Where(header => MarksASecret(header.Key))) + { + foreach (var value in header.Value) yield return CredentialOf(value); + } + + foreach (var needle in FromUrl(request.RequestUri?.OriginalString)) yield return needle; + } + + /// The long enough to be one (), de-duplicated. + public static IReadOnlyList Usable(IEnumerable needles) => needles.OfType().Where(needle => needle.Length >= MinimumLength).Distinct(StringComparer.Ordinal).ToList(); + + /// A redactor over the ; when none is. + public static SecretRedactor Redactor(IEnumerable needles) => Usable(needles) is { Count: > 0 } usable ? new SecretRedactor(usable) : SecretRedactor.None; + + /// + /// — an answer an upstream gave a request: an error body, a refusal message — with every + /// credential in struck wherever the upstream echoed it. For the call + /// sites that read a refusal's body THEMSELVES (a raw-HTTP provider call) and hold the token they sent; null stays null. + /// + [return: NotNullIfNotNull(nameof(text))] + public static string? Strike(string? text, IEnumerable presented) => text is null ? null : Redactor(presented).Redact(text); + + private static string CredentialOf(string headerValue) + { + var separator = headerValue.IndexOf(' '); + + return separator < 0 ? headerValue : headerValue[(separator + 1)..].Trim(); + } +} diff --git a/backend/src/CodeSpace.Core/Services/Redaction/CredentialUrl.cs b/backend/src/CodeSpace.Core/Services/Redaction/CredentialUrl.cs new file mode 100644 index 000000000..9f4708486 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Redaction/CredentialUrl.cs @@ -0,0 +1,117 @@ +namespace CodeSpace.Core.Services.Redaction; + +/// +/// A URL that may carry credentials — a model gateway's base URL with user:key@host or ?api-key= — seen two +/// ways: as a DISPLAY form safe to show any reader (the credential slots masked, everything else verbatim), and as the +/// ENDPOINT a stored key is bound to (scheme, host and port: where the key is sent). +/// +/// The credential slots are the ones strikes from echoed text: the whole +/// userinfo, and each query value whose name marks it a secret. The +/// display form is computed on the raw string rather than through , so a URL with nothing to mask reads +/// back exactly as it was stored — no host lower-casing, no trailing slash — and a URL cannot parse +/// still has its userinfo masked. +/// +public static class CredentialUrl +{ + /// What a credential slot reads as in the display form. + public const string Mask = "***"; + + /// The display form: userinfo and secret-named query values replaced by ; null/blank and a URL with no credential slot verbatim. + public static string? Masked(string? url) + { + if (string.IsNullOrEmpty(url)) return url; + + var masked = url; + + foreach (var slot in CredentialSlots(url).OrderByDescending(slot => slot.Start)) masked = masked[..slot.Start] + MaskFor(url.Substring(slot.Start, slot.Length)) + masked[(slot.Start + slot.Length)..]; + + return masked; + } + + /// Whether a credential slot of still holds the — a display form sent back with something else changed, which would store the placeholder as the credential. + public static bool CarriesMask(string? url) => !string.IsNullOrEmpty(url) && CredentialSlots(url).Any(slot => url.AsSpan(slot.Start, slot.Length).Contains(Mask.AsSpan(), StringComparison.Ordinal)); + + /// + /// The URL an update stores for a field the read side shows masked: the display form sent + /// back exactly as it was shown means "unchanged", so the stored URL — with its real embedded credentials — is kept, + /// and an edit form or API client that PUTs back what it GETs keeps working. A left in a credential + /// slot of any OTHER submitted URL would store the placeholder as the credential itself, so it is refused: the caller + /// re-types the credentials or removes them. Anything else, null included, is taken as submitted. + /// + public static string? KeepStoredIfMasked(string? stored, string? submitted) + { + if (submitted is not null && submitted == Masked(stored)) return stored; + + if (CarriesMask(submitted)) + throw new ArgumentException($"The URL still carries the masked placeholder ({Mask}) where a credential belongs. Send it back unchanged to keep the stored URL, or enter the embedded credentials in full (or remove them)."); + + return submitted; + } + + /// + /// Whether two base URLs name the same endpoint — the same scheme, host and port, which is where a key presented to + /// either one is sent. Path, query and userinfo may differ. Blank on both sides is the provider default on both sides. + /// A value that is not an absolute http(s) URL only matches itself. + /// + public static bool SameEndpoint(string? stored, string? next) + { + if (string.IsNullOrWhiteSpace(stored) && string.IsNullOrWhiteSpace(next)) return true; + + if (HttpUri(stored) is not { } a || HttpUri(next) is not { } b) return string.Equals(stored?.Trim(), next?.Trim(), StringComparison.Ordinal); + + return Uri.Compare(a, b, UriComponents.Scheme | UriComponents.Host | UriComponents.StrongPort, UriFormat.SafeUnescaped, StringComparison.OrdinalIgnoreCase) == 0; + } + + /// The endpoint alone — scheme://host:port, never userinfo, path or query — for a log line about where a key is sent. + public static string Endpoint(string? url) + { + if (string.IsNullOrWhiteSpace(url)) return "(provider default)"; + + return HttpUri(url) is { } parsed ? $"{parsed.Scheme}://{parsed.IdnHost}:{parsed.Port}" : "(unparseable)"; + } + + private static Uri? HttpUri(string? url) => Uri.TryCreate(url?.Trim(), UriKind.Absolute, out var parsed) && (parsed.Scheme == Uri.UriSchemeHttp || parsed.Scheme == Uri.UriSchemeHttps) ? parsed : null; + + /// A userinfo slot keeps its user:password shape; a query value is one opaque token. + private static string MaskFor(string slot) => slot.Contains(':') ? $"{Mask}:{Mask}" : Mask; + + /// The (start, length) of every credential slot in the raw string: the userinfo, then each non-empty secret-named query value. + private static IEnumerable<(int Start, int Length)> CredentialSlots(string url) + { + var (authorityStart, authorityEnd) = AuthorityBounds(url); + var at = url.AsSpan(authorityStart, authorityEnd - authorityStart).LastIndexOf('@'); + + if (at > 0) yield return (authorityStart, at); + + foreach (var slot in SecretQueryValues(url, authorityEnd)) yield return slot; + } + + /// Where the authority (userinfo@host:port) sits: after scheme:// (or at the start when there is none), up to the first /, ? or #. + private static (int Start, int End) AuthorityBounds(string url) + { + var scheme = url.IndexOf("://", StringComparison.Ordinal); + var start = scheme < 0 ? 0 : scheme + 3; + var end = url.IndexOfAny(['/', '?', '#'], start); + + return (start, end < 0 ? url.Length : end); + } + + private static IEnumerable<(int Start, int Length)> SecretQueryValues(string url, int authorityEnd) + { + var query = url.IndexOf('?', authorityEnd); + if (query < 0) yield break; + + var fragment = url.IndexOf('#', query); + var end = fragment < 0 ? url.Length : fragment; + + for (var pairStart = query + 1; pairStart < end;) + { + var pairEnd = url.IndexOf('&', pairStart, end - pairStart) is var amp and >= 0 ? amp : end; + var equals = url.IndexOf('=', pairStart, pairEnd - pairStart); + + if (equals > pairStart && equals + 1 < pairEnd && CredentialNeedles.MarksASecret(Uri.UnescapeDataString(url[pairStart..equals]))) yield return (equals + 1, pairEnd - equals - 1); + + pairStart = pairEnd + 1; + } + } +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmApiException.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmApiException.cs index 664107432..f3539e430 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmApiException.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmApiException.cs @@ -60,7 +60,7 @@ public LlmApiException(string provider, int? statusCode, LlmErrorCategory catego /// The machine-actionable failure category callers branch on. public LlmErrorCategory Category { get; } - /// The raw provider error body (the rate-limit / context-length / auth message), shown verbatim to operators. + /// The provider error body (the rate-limit / context-length / auth message), shown to operators — verbatim but for any credential the request presented, which the transport strikes before this exists. public string ProviderMessage { get; } /// The honored Retry-After delay when the provider supplied one (a 429/503), else null. diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmHttpTransport.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmHttpTransport.cs index 53a848842..0932e9c7c 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmHttpTransport.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/LlmHttpTransport.cs @@ -2,6 +2,7 @@ using System.Net.Http.Json; using System.Runtime.CompilerServices; using System.Text.Json; +using CodeSpace.Core.Services.Redaction; namespace CodeSpace.Core.Services.Workflows.Llm; @@ -50,7 +51,7 @@ public static async Task SendForJsonAsync(HttpClient http, HttpRequestMess var status = (int)response.StatusCode; var errorBody = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); - throw MarkIfAmbiguousSendPreceded(request, new LlmApiException(provider, status, LlmApiException.Classify(status, errorBody), errorBody, RetryAfterOf(response))); + throw MarkIfAmbiguousSendPreceded(request, new LlmApiException(provider, status, LlmApiException.Classify(status, errorBody), WithoutPresentedCredentials(request, errorBody), RetryAfterOf(response))); } try @@ -97,7 +98,7 @@ public static async IAsyncEnumerable StreamSseAsync(HttpClient http var status = (int)response.StatusCode; var errorBody = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); - throw MarkIfAmbiguousSendPreceded(request, new LlmApiException(provider, status, LlmApiException.Classify(status, errorBody), errorBody, RetryAfterOf(response))); + throw MarkIfAmbiguousSendPreceded(request, new LlmApiException(provider, status, LlmApiException.Classify(status, errorBody), WithoutPresentedCredentials(request, errorBody), RetryAfterOf(response))); } await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); @@ -128,6 +129,16 @@ public static StringContent JsonBody(T body, JsonSerializerOptions? options = return content; } + /// + /// The gateway's error body with every value THIS request presented struck out — its credential headers and the + /// credential parts of its URL (). An upstream that echoes the key it + /// rejected, or a proxy that reflects request headers, would otherwise hand the write-only key to every reader of + /// the failure: the node, interaction and external-call failure records, and the run a Viewer opens. Struck HERE, + /// before the exception exists, so no consumer downstream can forget to. The category is classified from the raw + /// body by the caller — a struck echo never changes the retry/degrade decision. + /// + private static string WithoutPresentedCredentials(HttpRequestMessage request, string errorBody) => CredentialNeedles.Redactor(CredentialNeedles.FromRequest(request)).Redact(errorBody); + /// Mark as riding over a possibly-billed earlier attempt when a PRECEDING attempt of this same (Polly-retried) request was sent and got no response back — see . This is what keeps LlmBudgetGuard.ObservedNoSpend holding the reservation instead of reading THIS attempt's own — possibly clean — outcome as proof the call spent nothing. private static LlmApiException MarkIfAmbiguousSendPreceded(HttpRequestMessage request, LlmApiException fault) => request.Options.TryGetValue(PhysicalLlmAccountingHandler.AmbiguousSendKey, out var marker) && marker.Occurred ? fault.AfterBilledAttempt() : fault; diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingLLMClientDecorator.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingLLMClientDecorator.cs index ecefe17e2..64bf23c5b 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingLLMClientDecorator.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingLLMClientDecorator.cs @@ -1,8 +1,10 @@ using System.Text; using System.Text.Json; using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Redaction; using CodeSpace.Core.Services.RunData; using CodeSpace.Core.Services.Workflows.Runtime; +using CodeSpace.Messages.Agents; using CodeSpace.Messages.Contracts; using CodeSpace.Messages.Constants; using Serilog; @@ -76,7 +78,7 @@ await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionStarted, correlat { // The caller token is commonly cancelled by the failure itself. Terminal capture gets one independent, // fail-open attempt so cancellation does not erase the interaction's final fact. - var failureRecorded = await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionFailed, correlationId, () => Task.FromResult(FailedPayload(scope, Provider, ex)), CancellationToken.None).ConfigureAwait(false); + var failureRecorded = await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionFailed, correlationId, () => Task.FromResult(FailedPayload(scope, Provider, ex, request.Credential)), CancellationToken.None).ConfigureAwait(false); if (failureRecorded && declared) await MarkCapturePresentAsync(scope).ConfigureAwait(false); throw; @@ -212,7 +214,7 @@ protected static JsonElement CompletedPayload(LlmCallScope scope, string provide output, }); - protected static JsonElement FailedPayload(LlmCallScope scope, string provider, Exception ex) + protected static JsonElement FailedPayload(LlmCallScope scope, string provider, Exception ex, ResolvedModelCredential? credential) { var category = ex is LlmApiException llm ? llm.Category.ToString() : null; var failureKind = ex switch @@ -222,10 +224,21 @@ protected static JsonElement FailedPayload(LlmCallScope scope, string provider, _ => "exception", }; - var error = scope.CaptureRedactor is null ? ex.Message : Observe(scope, scope.CaptureRedactor.Redact(ex.Message)); + var withoutCredential = WithoutCallCredential(scope, credential, ex.Message); + var error = scope.CaptureRedactor is null ? withoutCredential : Observe(scope, scope.CaptureRedactor.Redact(withoutCredential)); return JsonSerializer.SerializeToElement(new { kind = scope.Kind, accountingSource = scope.NativeModelCallId is not null ? Budget.BudgetLedger.PhysicalLlmSource : null, provider, error, category, failureKind }); } + /// + /// The failure text with the call's OWN credential struck — its key and the credential parts of its base URL + /// (). The transport already strikes an upstream's echo before the + /// typed failure exists; this backstop covers a failure raised anywhere else that still quotes it, so the + /// write-only key never reaches the interaction tape a Viewer reads. Folded into the call's masking observation like + /// every other capture-side redaction. + /// + private static string? WithoutCallCredential(LlmCallScope scope, ResolvedModelCredential? credential, string? text) => + CredentialNeedles.Usable(CredentialNeedles.FromCredential(credential)) is { Count: > 0 } needles ? Observe(scope, new PersistenceSecretRedactor(needles).Redact(text)) : text; + /// Fold one redaction into THIS call's masking observation and hand its value back, so the flag the presence delta carries is a fact about the bytes that reached storage rather than about the configuration. A scope minted outside observes nothing and reads back verbatim, which is the conservative answer. protected static T Observe(LlmCallScope scope, PersistenceRedaction redaction) => scope.Masking is { } masking ? masking.Observe(redaction) : redaction.Value; diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStreamingStructuredLLMClientDecorator.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStreamingStructuredLLMClientDecorator.cs index 4d7708fff..6456f59a6 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStreamingStructuredLLMClientDecorator.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStreamingStructuredLLMClientDecorator.cs @@ -93,7 +93,7 @@ await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionStarted, correlat if (!pending.IsEmpty) await RecordPendingDeltaAsync(scope, correlationId, pending, DeltaFlush.Cut, CancellationToken.None).ConfigureAwait(false); - var failureRecorded = await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionFailed, correlationId, () => Task.FromResult(FailedPayload(scope, Provider, ex)), CancellationToken.None).ConfigureAwait(false); + var failureRecorded = await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionFailed, correlationId, () => Task.FromResult(FailedPayload(scope, Provider, ex, request.Credential)), CancellationToken.None).ConfigureAwait(false); if (failureRecorded && declared) await MarkCapturePresentAsync(scope).ConfigureAwait(false); throw; diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStructuredLLMClientDecorator.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStructuredLLMClientDecorator.cs index 65e0e55cf..c146c9c0c 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStructuredLLMClientDecorator.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/RecordingStructuredLLMClientDecorator.cs @@ -55,7 +55,7 @@ await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionStarted, correlat } catch (Exception ex) { - var failureRecorded = await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionFailed, correlationId, () => Task.FromResult(FailedPayload(scope, Provider, ex)), CancellationToken.None).ConfigureAwait(false); + var failureRecorded = await SafeRecordAsync(scope, WorkflowRunRecordTypes.InteractionFailed, correlationId, () => Task.FromResult(FailedPayload(scope, Provider, ex, request.Credential)), CancellationToken.None).ConfigureAwait(false); if (failureRecorded && declared) await MarkCapturePresentAsync(scope).ConfigureAwait(false); throw; diff --git a/backend/src/CodeSpace.Messages/Dtos/ModelCredentials/ModelCredentialSummary.cs b/backend/src/CodeSpace.Messages/Dtos/ModelCredentials/ModelCredentialSummary.cs index 493540180..7d6af5821 100644 --- a/backend/src/CodeSpace.Messages/Dtos/ModelCredentials/ModelCredentialSummary.cs +++ b/backend/src/CodeSpace.Messages/Dtos/ModelCredentials/ModelCredentialSummary.cs @@ -5,7 +5,7 @@ namespace CodeSpace.Messages.Dtos.ModelCredentials; /// /// A team's model credential as shown in the management UI — NEVER carries the secret. The key is /// surfaced only as a masked hint (last 4 chars, derived transiently on read, then discarded), exactly -/// like the OpenAI / Anthropic consoles. is non-secret config and is shown verbatim. +/// like the OpenAI / Anthropic consoles. is shown with its embedded credentials masked. /// public sealed record ModelCredentialSummary { @@ -27,7 +27,11 @@ public sealed record ModelCredentialSummary /// public bool KeyUnreadable { get; init; } - /// Non-secret base-URL override (gateway / self-hosted endpoint), shown as-is. Null = the provider default. + /// + /// The base-URL override (gateway / self-hosted endpoint), with any credential it embeds masked — userinfo and + /// secret-named query values read *** (https://***:***@gw/v1?api-key=***); a URL embedding none reads + /// exactly as stored. Sending this exact value back on update keeps the stored URL. Null = the provider default. + /// public string? BaseUrl { get; init; } public required CredentialStatus Status { get; init; } diff --git a/backend/src/CodeSpace.Messages/Exceptions/ModelEndpointKeyRequiredException.cs b/backend/src/CodeSpace.Messages/Exceptions/ModelEndpointKeyRequiredException.cs new file mode 100644 index 000000000..f995db13e --- /dev/null +++ b/backend/src/CodeSpace.Messages/Exceptions/ModelEndpointKeyRequiredException.cs @@ -0,0 +1,27 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Messages.Exceptions; + +/// +/// Thrown when an update moves a model credential's endpoint — its base URL's scheme, host or port — without entering +/// the API key again. A stored key is write-only: nobody can read it back, so the only thing that could carry it to a new +/// host is a repoint that kept it, and every reflection, probe and run after that would present it there. Remedy: send +/// the key in the same update as the new endpoint (or leave the endpoint where it is). A path-only change, or a keyless +/// credential, needs no key. +/// +public sealed class ModelEndpointKeyRequiredException : Exception, IFailure +{ + public FailureKind Kind => FailureKind.Invalid; + + public string Code => FailureCodes.ModelEndpointKeyRequired; + + public IReadOnlyDictionary? Details => new Dictionary { ["credentialId"] = CredentialId }; + + public Guid CredentialId { get; } + + public ModelEndpointKeyRequiredException(Guid credentialId) + : base("Changing this credential's endpoint (its scheme, host or port) needs the API key entered again in the same update — a stored key is only ever sent to the endpoint it was entered for.") + { + CredentialId = credentialId; + } +} diff --git a/backend/src/CodeSpace.Messages/Exceptions/ProviderApiException.cs b/backend/src/CodeSpace.Messages/Exceptions/ProviderApiException.cs index 5ab65c32d..937c069d0 100644 --- a/backend/src/CodeSpace.Messages/Exceptions/ProviderApiException.cs +++ b/backend/src/CodeSpace.Messages/Exceptions/ProviderApiException.cs @@ -47,7 +47,7 @@ public ProviderApiException(ProviderKind providerKind, int statusCode, string op /// The capability method that failed (e.g. "ListPullRequestsAsync"). Surfaced to the operator for context. public string OperationName { get; } - /// Raw provider error string (e.g. Octokit's "Not Found"). Stripped from logs / shown verbatim to operators. + /// The provider's error string (e.g. Octokit's "Not Found"), shown to the caller — with every token the instance presented struck and the SDK's trailing "Original call: <url>" dropped by the translation that raises this. public string ProviderMessage { get; } private static string BuildMessage(ProviderKind kind, int statusCode, string operation, string providerMessage) diff --git a/backend/src/CodeSpace.Messages/Exceptions/ProviderEndpointCredentialsBoundException.cs b/backend/src/CodeSpace.Messages/Exceptions/ProviderEndpointCredentialsBoundException.cs new file mode 100644 index 000000000..cdbffa43a --- /dev/null +++ b/backend/src/CodeSpace.Messages/Exceptions/ProviderEndpointCredentialsBoundException.cs @@ -0,0 +1,31 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Messages.Exceptions; + +/// +/// Thrown when an edit moves a git provider connection's endpoint — the scheme, host or port of its API or base URL — +/// while credentials are still connected through it. A connected token is write-only: nobody can read it back, so the +/// only thing that could carry it to a new host is a repoint that kept it, and every repository read, git node, webhook +/// registration and OAuth refresh after that would present it there. The tokens belong to the members who connected +/// them, so the editor cannot re-enter them the way a model key is re-entered; remedy: disconnect them first (each +/// member reconnects against the new host), or add the new host as a provider of its own. +/// +public sealed class ProviderEndpointCredentialsBoundException : Exception, IFailure +{ + public FailureKind Kind => FailureKind.PreconditionRequired; + + public string Code => FailureCodes.ProviderEndpointCredentialsBound; + + public IReadOnlyDictionary? Details => new Dictionary { ["providerInstanceId"] = ProviderInstanceId, ["boundCredentialCount"] = BoundCredentialCount }; + + public Guid ProviderInstanceId { get; } + + public int BoundCredentialCount { get; } + + public ProviderEndpointCredentialsBoundException(Guid providerInstanceId, int boundCredentialCount) + : base($"Changing this provider's host (its scheme, host or port) is refused while {boundCredentialCount} connected credential(s) would follow it — a connected token is only ever sent to the host it was connected for. Disconnect them first (members reconnect against the new host), or add the new host as its own provider.") + { + ProviderInstanceId = providerInstanceId; + BoundCredentialCount = boundCredentialCount; + } +} diff --git a/backend/src/CodeSpace.Messages/Exceptions/ProviderEndpointSecretRequiredException.cs b/backend/src/CodeSpace.Messages/Exceptions/ProviderEndpointSecretRequiredException.cs new file mode 100644 index 000000000..2742aa329 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Exceptions/ProviderEndpointSecretRequiredException.cs @@ -0,0 +1,27 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Messages.Exceptions; + +/// +/// Thrown when an edit moves a git provider connection's endpoint — the scheme, host or port of its API or base URL — +/// without entering its stored OAuth client secret again. The secret is write-only and is sent with every token +/// exchange, refresh and revocation, to endpoints derived from the base URL; kept across a repoint, it would go to the +/// new host on the next sign-in. Remedy: send the client secret in the same update as the new endpoint (an OAuth app +/// belongs to one host, so this is normally the new host's app's secret, with its client ID). +/// +public sealed class ProviderEndpointSecretRequiredException : Exception, IFailure +{ + public FailureKind Kind => FailureKind.Invalid; + + public string Code => FailureCodes.ProviderEndpointSecretRequired; + + public IReadOnlyDictionary? Details => new Dictionary { ["providerInstanceId"] = ProviderInstanceId }; + + public Guid ProviderInstanceId { get; } + + public ProviderEndpointSecretRequiredException(Guid providerInstanceId) + : base("Changing this provider's host (its scheme, host or port) needs the OAuth client secret entered again in the same update — a stored secret is only ever sent to the host it was entered for.") + { + ProviderInstanceId = providerInstanceId; + } +} diff --git a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs index 8d1c572e1..54de365e8 100644 --- a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs +++ b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs @@ -55,6 +55,15 @@ public static class FailureCodes /// Something tried to authenticate through a credential that was disconnected — revoked, its token material destroyed. Remedy: reconnect the provider (bound repositories follow their owner's reconnect) or re-link to an active credential; a retry as-is reads the same dead row. public const string CredentialDisconnected = "credential_disconnected"; + /// An update moved a model credential's endpoint (scheme, host or port) without entering its key again. Remedy: send the key with the new endpoint; a stored key never follows a repoint on its own. + public const string ModelEndpointKeyRequired = "model_endpoint_key_required"; + + /// An edit moved a git provider connection's endpoint (the scheme, host or port of its API or base URL) while credentials are still connected through it. Remedy: disconnect them first (members reconnect against the new host), or add the new host as its own provider; a connected token never follows a repoint. + public const string ProviderEndpointCredentialsBound = "provider_endpoint_credentials_bound"; + + /// An edit moved a git provider connection's endpoint without entering its stored OAuth client secret again. Remedy: send the secret (normally the new host's OAuth app's) with the new endpoint. + public const string ProviderEndpointSecretRequired = "provider_endpoint_secret_required"; + // ── Workflows and runs ───────────────────────────────────────────────────────── public const string WorkflowDefinitionInvalid = "workflow_definition_invalid"; public const string TaskRouteConfirmationRequired = "task_route_confirmation_required"; diff --git a/backend/tests/CodeSpace.E2ETests/Agents/ModelKeyExposureE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Agents/ModelKeyExposureE2ETests.cs new file mode 100644 index 000000000..9edc43565 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Agents/ModelKeyExposureE2ETests.cs @@ -0,0 +1,188 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text; +using System.Text.Json; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Dtos.ModelCredentials; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.E2ETests.Agents; + +/// +/// A team's model key is write-only to every member — nobody reads it back, and nobody can make the platform carry it +/// somewhere it was not entered for. Driven as a client drives it: the real HTTP surface (routing, JWT, X-Team-Id, +/// model binding, the controller, the GlobalExceptionFilter, MediatR) over real Postgres, with loopback gateways that +/// record every connection they accept and echo the credentials they are presented. +/// +/// Fidelity: HIGH. The production ModelCredentialService, the production reflector over its production +/// named client, the production engine and OpenAiClient — only the gateways are stubs, and they are real sockets. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class ModelKeyExposureE2ETests : IClassFixture +{ + private readonly TaskLaunchApiFactory _factory; + private readonly TenantHarness _tenants; + + public ModelKeyExposureE2ETests(TaskLaunchApiFactory factory) + { + _factory = factory; + _tenants = new TenantHarness(factory); + } + + [Fact] + public async Task An_admin_cannot_repoint_a_credential_to_take_its_key_somewhere_else() + { + var key = Canary("OWNER-KEY"); + await using var legit = LoopbackResponder.Start(_ => ModelList()); + await using var attacker = LoopbackResponder.Start(_ => ModelList()); + var world = await SeedAsync(); + var id = await AddCredentialAsync(world.Owner, world.Team, key, $"http://127.0.0.1:{legit.Port}/v1"); + + var repoint = await _tenants.SendAsync(HttpMethod.Put, $"/api/model-credentials/{id}", world.Admin, world.Team, new { displayName = "gateway", baseUrl = $"http://127.0.0.1:{attacker.Port}/v1" }); + repoint.StatusCode.ShouldBe(HttpStatusCode.BadRequest, await TenantHarness.DescribeAsync(repoint)); + (await repoint.Content.ReadAsStringAsync()).ShouldContain("model_endpoint_key_required"); + + var refresh = await _tenants.SendAsync(HttpMethod.Post, $"/api/model-credentials/{id}/models/refresh", world.Admin, world.Team); + refresh.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(refresh)); + + attacker.Accepted.ShouldBe(0, "the refused repoint stored nothing, so the refresh after it went to the endpoint the key was entered for — check the credential's base_url row if this fails"); + legit.Requests.ShouldContain(head => head.Contains($"Bearer {key}"), "fixture check: the refresh really presented the key, to its own endpoint"); + + var moved = await _tenants.SendAsync(HttpMethod.Put, $"/api/model-credentials/{id}", world.Admin, world.Team, new { displayName = "gateway", apiKey = "sk-admins-own-key-0123456789", baseUrl = $"http://127.0.0.1:{attacker.Port}/v1" }); + moved.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(moved)); + (await _tenants.SendAsync(HttpMethod.Post, $"/api/model-credentials/{id}/models/refresh", world.Admin, world.Team)).StatusCode.ShouldBe(HttpStatusCode.OK); + + attacker.Requests.ShouldNotBeEmpty("fixture check: a repoint WITH a key is the supported way to move a credential"); + attacker.Requests.ShouldAllBe(head => !head.Contains(key), "the new endpoint only ever sees the key entered with the move"); + } + + [Theory] + [InlineData(TeamRole.Owner)] + [InlineData(TeamRole.Admin)] + [InlineData(TeamRole.Member)] + [InlineData(TeamRole.Viewer)] + public async Task Every_member_reads_the_base_url_with_its_credentials_masked(TeamRole role) + { + var password = Canary("URL-PASSWORD"); + var queryKey = Canary("URL-QUERY-KEY"); + var world = await SeedAsync(); + await AddCredentialAsync(world.Owner, world.Team, Canary("KEY"), $"https://svc:{password}@gw.example/v1?api-key={queryKey}&api-version=2024-01-01"); + + var list = await _tenants.SendAsync(HttpMethod.Get, "/api/model-credentials", world.As(role), world.Team); + var body = await list.Content.ReadAsStringAsync(); + + list.StatusCode.ShouldBe(HttpStatusCode.OK, body); + body.ShouldNotContain(password); + body.ShouldNotContain(queryKey); + JsonSerializer.Deserialize(body, Json)!.ShouldHaveSingleItem().BaseUrl.ShouldBe("https://***:***@gw.example/v1?api-key=***&api-version=2024-01-01"); + } + + [Fact] + public async Task An_edit_form_that_sends_back_what_it_was_shown_keeps_the_stored_url() + { + var stored = $"https://svc:{Canary("URL-PASSWORD")}@gw.example/v1"; + var world = await SeedAsync(); + var id = await AddCredentialAsync(world.Owner, world.Team, Canary("KEY"), stored); + var shown = (await (await _tenants.SendAsync(HttpMethod.Get, "/api/model-credentials", world.Admin, world.Team)).Content.ReadFromJsonAsync(Json))!.Single(); + + var save = await _tenants.SendAsync(HttpMethod.Put, $"/api/model-credentials/{id}", world.Admin, world.Team, new { displayName = "renamed", apiKey = (string?)null, baseUrl = shown.BaseUrl }); + + save.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(save)); + (await _tenants.DbAsync(db => db.ModelCredential.AsNoTracking().SingleAsync(c => c.Id == id))).BaseUrl.ShouldBe(stored, "the masked round-trip keeps the credentials the platform needs to reach the gateway"); + } + + [Fact] + public async Task A_viewer_reads_a_failed_run_without_the_key_the_gateway_echoed() + { + var key = Canary("RUN-KEY"); + await using var gateway = LoopbackResponder.Start(EchoTheBearer); + var world = await SeedAsync(); + var id = await AddCredentialAsync(world.Owner, world.Team, key, $"http://127.0.0.1:{gateway.Port}/v1"); + (await _tenants.SendAsync(HttpMethod.Post, $"/api/model-credentials/{id}/models", world.Owner, world.Team, new { modelId = "gpt-echo" })).StatusCode.ShouldBe(HttpStatusCode.OK); + + var runId = await LaunchLlmRunAsync(world); + await _factory.JobClient.DrainAsync(); + + gateway.Requests.ShouldContain(head => head.Contains(key), "fixture check: the run presented the key to the gateway, whose 401 echoed it"); + foreach (var path in new[] { $"/api/workflows/runs/{runId}", $"/api/workflows/runs/{runId}/records", $"/api/workflows/runs/{runId}/timeline" }) + { + var response = await _tenants.SendAsync(HttpMethod.Get, path, world.Viewer, world.Team); + var body = await response.Content.ReadAsStringAsync(); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, $"{path}: {body}"); + body.ShouldNotContain(key, Case.Sensitive, $"{path} handed a Viewer the echoed model key"); + } + } + + private async Task LaunchLlmRunAsync(World world) + { + var workflow = new + { + name = $"echo-{Guid.NewGuid():N}"[..20], enabled = true, activations = Array.Empty(), + definition = new + { + schemaVersion = 1, + nodes = new object[] + { + new { id = "start", typeKey = "trigger.manual", label = "start", config = new { }, inputs = new { } }, + new { id = "llm", typeKey = "llm.complete", label = "llm", config = new { provider = "Custom", model = "gpt-echo" }, inputs = new { userPrompt = "hello" } }, + new { id = "end", typeKey = "builtin.terminal", label = "end", config = new { }, inputs = new { } }, + }, + edges = new object[] { new { from = "start", to = "llm" }, new { from = "llm", to = "end" } }, + }, + }; + + var created = await _tenants.SendAsync(HttpMethod.Post, "/api/workflows", world.Owner, world.Team, workflow); + created.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(created)); + var workflowId = (await created.Content.ReadFromJsonAsync()).GetProperty("id").GetGuid(); + + var launched = await _tenants.SendAsync(HttpMethod.Post, $"/api/workflows/{workflowId}/run", world.Owner, world.Team, new { }); + launched.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(launched)); + + return (await launched.Content.ReadFromJsonAsync()).GetProperty("runId").GetGuid(); + } + + private async Task AddCredentialAsync(Guid owner, Guid team, string key, string baseUrl) + { + var added = await _tenants.SendAsync(HttpMethod.Post, "/api/model-credentials", owner, team, new { provider = "Custom", displayName = "gateway", apiKey = key, baseUrl }); + added.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(added)); + + return (await added.Content.ReadFromJsonAsync()).GetProperty("id").GetGuid(); + } + + private async Task SeedAsync() + { + var owner = await _tenants.AccountAsync(); + var admin = await _tenants.AccountAsync(); + var member = await _tenants.AccountAsync(); + var viewer = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner), (admin, TeamRole.Admin), (member, TeamRole.Member), (viewer, TeamRole.Viewer)); + + return new World(team, owner, admin, member, viewer); + } + + private static string Canary(string label) => $"CANARY-{label}-{Guid.NewGuid():N}"; + + private static string ModelList() => Respond(HttpStatusCode.OK, """{"object":"list","data":[{"id":"m1","object":"model"}]}"""); + + /// A gateway that rejects the key it was presented and quotes it back, the way a careless 401 does. + private static string EchoTheBearer(string head) + { + var presented = head.Split("\r\n").FirstOrDefault(line => line.StartsWith("Authorization:", StringComparison.OrdinalIgnoreCase)) ?? ""; + + return Respond(HttpStatusCode.Unauthorized, JsonSerializer.Serialize(new { error = new { message = $"Invalid API key provided: {presented}", type = "invalid_request_error" } })); + } + + private static string Respond(HttpStatusCode status, string body) => $"HTTP/1.1 {(int)status} {status}\r\nContent-Type: application/json\r\nContent-Length: {Encoding.UTF8.GetByteCount(body)}\r\nConnection: close\r\n\r\n{body}"; + + private static readonly JsonSerializerOptions Json = new(JsonSerializerDefaults.Web) { Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() } }; + + private sealed record World(Guid Team, Guid Owner, Guid Admin, Guid Member, Guid Viewer) + { + public Guid As(TeamRole role) => role switch { TeamRole.Owner => Owner, TeamRole.Admin => Admin, TeamRole.Member => Member, _ => Viewer }; + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Repositories/GitProviderErrorEchoE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Repositories/GitProviderErrorEchoE2ETests.cs new file mode 100644 index 000000000..800e41035 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Repositories/GitProviderErrorEchoE2ETests.cs @@ -0,0 +1,88 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text; +using System.Text.Json; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.E2ETests.Repositories; + +/// +/// A GitLab (or a proxy in front of it) whose error body quotes the token it was presented must not hand that token to +/// a Viewer browsing the repository. Driven through the real HTTP surface, the real GitLab provider over NGitLab, the +/// real resilience translation and the real GlobalExceptionFilter, against a loopback GitLab that answers every project +/// call with a 404 echoing the request's Authorization and PRIVATE-TOKEN. +/// +/// Fidelity: HIGH. Only GitLab is a stub, and it is a real socket the production client speaks HTTP to. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class GitProviderErrorEchoE2ETests : IClassFixture +{ + private readonly TenantHarness _tenants; + + public GitProviderErrorEchoE2ETests(TaskLaunchApiFactory factory) { _tenants = new TenantHarness(factory); } + + [Theory] + [InlineData("branches")] + [InlineData("tags")] + [InlineData("releases")] + [InlineData("pull-requests")] + [InlineData("issues")] + [InlineData("tree")] + public async Task A_viewer_never_reads_the_token_an_echoing_gitlab_quoted(string page) + { + var token = $"glpat-CANARY-{Guid.NewGuid():N}"; + await using var gitLab = LoopbackResponder.Start(EchoThePresentedToken); + var (viewer, team, repositoryId) = await SeedRepositoryAsync(token, gitLab.Port); + + var response = await _tenants.SendAsync(HttpMethod.Get, $"/api/repositories/{repositoryId}/{page}", viewer, team); + var body = await response.Content.ReadAsStringAsync(); + + gitLab.Requests.ShouldContain(head => head.Contains(token), "fixture check: the provider was presented the token, so its 404 quoted it"); + ((int)response.StatusCode).ShouldBeInRange(400, 499, body); + body.ShouldContain("Project Not Found", Case.Sensitive, "the provider's own words still reach the caller"); + body.ShouldNotContain(token, Case.Sensitive, $"/{page} reflected the presented token to a Viewer"); + body.ShouldNotContain("Original call", Case.Sensitive, "the SDK's internal request line is not the caller's to read"); + } + + private static string EchoThePresentedToken(string head) + { + var lines = head.Split("\r\n"); + string Header(string name) => lines.FirstOrDefault(line => line.StartsWith(name + ":", StringComparison.OrdinalIgnoreCase))?[(name.Length + 1)..].Trim() ?? ""; + var body = JsonSerializer.Serialize(new { message = $"404 Project Not Found (PRIVATE-TOKEN={Header("PRIVATE-TOKEN")}; Authorization={Header("Authorization")})" }); + + return $"HTTP/1.1 404 Not Found\r\nContent-Type: application/json\r\nContent-Length: {Encoding.UTF8.GetByteCount(body)}\r\nConnection: close\r\n\r\n{body}"; + } + + /// An Owner connects GitLab and adds a PAT through the API, as an operator would; the repository row is seeded directly so no bind round-trip to the stub is needed. + private async Task<(Guid Viewer, Guid Team, Guid RepositoryId)> SeedRepositoryAsync(string token, int port) + { + var owner = await _tenants.AccountAsync(); + var viewer = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner), (viewer, TeamRole.Viewer)); + + var instanceId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/provider-instances", owner, team, new { provider = "GitLab", displayName = "echoing gitlab", baseUrl = $"http://127.0.0.1:{port}" })); + var credentialId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/pat", owner, team, new { providerInstanceId = instanceId, displayName = "pat", token })); + var repositoryId = Guid.NewGuid(); + + await _tenants.DbAsync(async db => + { + db.Repository.Add(new Repository { Id = repositoryId, TeamId = team, ProviderInstanceId = instanceId, CredentialId = credentialId, ExternalId = "42", NamespacePath = "acme", Name = "web", FullPath = "acme/web", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "http://127.0.0.1/acme/web", Status = RepositoryStatus.Active, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + return await db.SaveChangesAsync(); + }); + + return (viewer, team, repositoryId); + } + + private static async Task CreatedIdAsync(HttpResponseMessage response) + { + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response)); + + return (await response.Content.ReadFromJsonAsync()).GetProperty("id").GetGuid(); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Repositories/ProviderInstanceRepointE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Repositories/ProviderInstanceRepointE2ETests.cs new file mode 100644 index 000000000..3997fc569 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Repositories/ProviderInstanceRepointE2ETests.cs @@ -0,0 +1,111 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.E2ETests.Repositories; + +/// +/// A connected token is only ever presented to the host it was connected for. An Admin may edit a git provider +/// connection, but a PATCH that moves its base or API URL to another host while a member's token is connected is refused +/// — so a Viewer browsing a repository afterwards still reaches the real GitLab, and the host the Admin named is never +/// contacted at all. And the connection's URLs reach every member masked where they embed credentials. +/// +/// Driven through the real HTTP surface (PATCH /api/provider-instances, GET /api/repositories/{id}/branches, +/// GET /api/provider-instances), the real GitLab provider over NGitLab and the real GlobalExceptionFilter, against two +/// loopback GitLabs — the legitimate one the token was connected for and an attacker's. +/// +/// Fidelity: HIGH. Only GitLab is a stub, and both are real sockets the production client would speak HTTP to. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class ProviderInstanceRepointE2ETests : IClassFixture +{ + private readonly TenantHarness _tenants; + + public ProviderInstanceRepointE2ETests(TaskLaunchApiFactory factory) { _tenants = new TenantHarness(factory); } + + [Theory] + [InlineData("baseUrl")] + [InlineData("apiUrl")] + public async Task An_admins_repoint_is_refused_and_the_owners_token_never_reaches_the_new_host(string field) + { + var token = $"glpat-REPOINT-CANARY-{Guid.NewGuid():N}"; + await using var legit = LoopbackResponder.Start(_ => LoopbackResponder.Ok("[]")); + await using var attacker = LoopbackResponder.Start(_ => LoopbackResponder.Ok("[]")); + var (admin, viewer, team, instanceId, repositoryId) = await SeedConnectedRepositoryAsync($"http://127.0.0.1:{legit.Port}", token); + + var repoint = await _tenants.SendAsync(HttpMethod.Patch, $"/api/provider-instances/{instanceId}", admin, team, Patch(field, $"http://127.0.0.1:{attacker.Port}")); + + repoint.StatusCode.ShouldBe(HttpStatusCode.PreconditionRequired, await TenantHarness.DescribeAsync(repoint)); + (await repoint.Content.ReadAsStringAsync()).ShouldContain("provider_endpoint_credentials_bound"); + + var browse = await _tenants.SendAsync(HttpMethod.Get, $"/api/repositories/{repositoryId}/branches", viewer, team); + + browse.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(browse)); + legit.Requests.ShouldContain(head => head.Contains(token), "fixture check: the Viewer's read presented the owner's token to the host it was connected for"); + attacker.Accepted.ShouldBe(0, $"the refused {field} repoint stored nothing, so no request went to the Admin's host — check provider_instance.{field} if this fails"); + } + + [Fact] + public async Task A_viewer_lists_the_connection_with_its_embedded_credentials_masked_and_a_masked_patch_keeps_them() + { + var canary = "CANARYPI" + Guid.NewGuid().ToString("N")[..12]; + var owner = await _tenants.AccountAsync(); + var viewer = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner), (viewer, TeamRole.Viewer)); + var instanceId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/provider-instances", owner, team, new { provider = "GitLab", displayName = "gitlab", baseUrl = $"https://svc:{canary}@gitlab.example", apiUrl = $"https://gitlab.example/api?private_token={canary}" })); + + var list = await _tenants.SendAsync(HttpMethod.Get, "/api/provider-instances", viewer, team); + var body = await list.Content.ReadAsStringAsync(); + + list.StatusCode.ShouldBe(HttpStatusCode.OK, body); + body.ShouldNotContain(canary, Case.Sensitive, "a Viewer read the credentials embedded in the connection's URLs"); + var listed = JsonDocument.Parse(body).RootElement.EnumerateArray().Single(p => p.GetProperty("id").GetGuid() == instanceId); + listed.GetProperty("baseUrl").GetString().ShouldBe("https://***:***@gitlab.example"); + listed.GetProperty("apiUrl").GetString().ShouldBe("https://gitlab.example/api?private_token=***"); + + var patch = await _tenants.SendAsync(HttpMethod.Patch, $"/api/provider-instances/{instanceId}", owner, team, new { displayName = "renamed", baseUrl = listed.GetProperty("baseUrl").GetString(), apiUrl = listed.GetProperty("apiUrl").GetString() }); + + patch.StatusCode.ShouldBe(HttpStatusCode.NoContent, await TenantHarness.DescribeAsync(patch)); + var stored = await _tenants.DbAsync(db => db.ProviderInstance.AsNoTracking().SingleAsync(p => p.Id == instanceId)); + stored.BaseUrl.ShouldBe($"https://svc:{canary}@gitlab.example", "sending the masked form back means unchanged — the stored credentials are kept"); + stored.ApiUrl.ShouldBe($"https://gitlab.example/api?private_token={canary}"); + stored.DisplayName.ShouldBe("renamed"); + } + + private static object Patch(string field, string url) => field == "baseUrl" ? new { baseUrl = url } : new { apiUrl = url }; + + /// The Owner connects GitLab and adds a PAT through the API, as an operator would; the repository row is seeded directly so no bind round-trip to the stub is needed. + private async Task<(Guid Admin, Guid Viewer, Guid Team, Guid InstanceId, Guid RepositoryId)> SeedConnectedRepositoryAsync(string gitLabUrl, string token) + { + var owner = await _tenants.AccountAsync(); + var admin = await _tenants.AccountAsync(); + var viewer = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner), (admin, TeamRole.Admin), (viewer, TeamRole.Viewer)); + var instanceId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/provider-instances", owner, team, new { provider = "GitLab", displayName = "gitlab", baseUrl = gitLabUrl })); + var credentialId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/pat", owner, team, new { providerInstanceId = instanceId, displayName = "owner pat", token })); + var repositoryId = Guid.NewGuid(); + + await _tenants.DbAsync(async db => + { + db.Repository.Add(new Repository { Id = repositoryId, TeamId = team, ProviderInstanceId = instanceId, CredentialId = credentialId, ExternalId = "42", NamespacePath = "acme", Name = "web", FullPath = "acme/web", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "http://127.0.0.1/acme/web", Status = RepositoryStatus.Active, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + return await db.SaveChangesAsync(); + }); + + return (admin, viewer, team, instanceId, repositoryId); + } + + private static async Task CreatedIdAsync(HttpResponseMessage response) + { + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response)); + + return (await response.Content.ReadFromJsonAsync()).GetProperty("id").GetGuid(); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Agents/ModelCredentialFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Agents/ModelCredentialFlowTests.cs index 8be5e036f..0077d4ff4 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Agents/ModelCredentialFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Agents/ModelCredentialFlowTests.cs @@ -6,6 +6,7 @@ using CodeSpace.Messages.Commands.ModelCredentials; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; using CodeSpace.Messages.Queries.ModelCredentials; using MediatR; using Microsoft.EntityFrameworkCore; @@ -96,10 +97,10 @@ public async Task A_credential_whose_key_cannot_be_decrypted_still_lists_with_a_ public async Task Update_rotates_the_key_only_when_one_is_supplied() { var (userId, teamId) = await SeedTeamAsync(); - var id = await SendAsync(userId, teamId, new AddModelCredentialCommand { Provider = "OpenAI", DisplayName = "Old name", ApiKey = "sk-old-aaaa" }); + var id = await SendAsync(userId, teamId, new AddModelCredentialCommand { Provider = "OpenAI", DisplayName = "Old name", ApiKey = "sk-old-aaaa", BaseUrl = "https://gw/v1" }); - // No key supplied → display name + base url change, key kept. - await SendAsync(userId, teamId, new UpdateModelCredentialCommand { Id = id, DisplayName = "New name", BaseUrl = "https://gw/v1" }); + // No key supplied → display name + base url path change (same endpoint), key kept. + await SendAsync(userId, teamId, new UpdateModelCredentialCommand { Id = id, DisplayName = "New name", BaseUrl = "https://gw/v2" }); (await DecryptKeyAsync(id)).ShouldBe("sk-old-aaaa", "a blank key on update keeps the existing one"); // Key supplied → rotated. @@ -111,6 +112,73 @@ public async Task Update_rotates_the_key_only_when_one_is_supplied() summary.KeyHint.ShouldBe("····bbbb"); } + // ── The stored key is bound to the endpoint it was entered for (scheme, host, port) ── + + [Fact] + public async Task Moving_the_endpoint_without_the_key_is_refused_and_changes_nothing() + { + var (userId, teamId) = await SeedTeamAsync(); + var id = await SendAsync(userId, teamId, new AddModelCredentialCommand { Provider = "OpenAI", DisplayName = "Gateway", ApiKey = "sk-bound-0123456789", BaseUrl = "https://gw.example/v1" }); + + var refusal = await Should.ThrowAsync(() => SendAsync(userId, teamId, new UpdateModelCredentialCommand { Id = id, DisplayName = "Repointed", BaseUrl = "https://attacker.example/v1" })); + + refusal.CredentialId.ShouldBe(id); + var row = await LoadAsync(id); + row.BaseUrl.ShouldBe("https://gw.example/v1", "a refused repoint stores nothing — the key still answers only to its own endpoint"); + row.DisplayName.ShouldBe("Gateway"); + (await DecryptKeyAsync(id)).ShouldBe("sk-bound-0123456789"); + } + + [Theory] + [InlineData("sk-bound-0123456789", "https://attacker.example/v1", "sk-entered-with-the-move")] // the key travels with the move because it was entered with it + [InlineData("sk-bound-0123456789", "https://gw.example/v2", null)] // path-only: same endpoint, key kept + [InlineData("sk-bound-0123456789", "https://GW.example:443/v1", null)] // same endpoint spelled differently + [InlineData(null, "https://other.example/v1", null)] // keyless: nothing to carry + public async Task An_endpoint_change_that_carries_no_stored_key_elsewhere_is_accepted(string? addedKey, string nextUrl, string? updateKey) + { + var (userId, teamId) = await SeedTeamAsync(); + var id = await SendAsync(userId, teamId, new AddModelCredentialCommand { Provider = "Custom", DisplayName = "Gateway", ApiKey = addedKey, BaseUrl = "https://gw.example/v1" }); + + await SendAsync(userId, teamId, new UpdateModelCredentialCommand { Id = id, DisplayName = "Gateway", BaseUrl = nextUrl, ApiKey = updateKey }); + + var row = await LoadAsync(id); + row.BaseUrl.ShouldBe(nextUrl); + if ((updateKey ?? addedKey) is { } expectedKey) (await DecryptKeyAsync(id)).ShouldBe(expectedKey); + else row.EncryptedApiKey.ShouldBeNull(); + } + + [Fact] + public async Task The_list_masks_embedded_credentials_and_sending_the_masked_value_back_keeps_the_stored_url() + { + const string stored = "https://svc:CANARY-MC-PW-01@gw.example/v1?api-key=CANARY-MC-Q-02&api-version=2024-01-01"; + var (userId, teamId) = await SeedTeamAsync(); + var id = await SendAsync(userId, teamId, new AddModelCredentialCommand { Provider = "Custom", DisplayName = "Embedded", ApiKey = "sk-embedded-0123456789", BaseUrl = stored }); + + var shown = (await SendAsync(userId, teamId, new ListModelCredentialsQuery())).ShouldHaveSingleItem().BaseUrl; + shown.ShouldBe("https://***:***@gw.example/v1?api-key=***&api-version=2024-01-01"); + + // What an edit form does: PUT back exactly what it was shown, with only the name changed. + await SendAsync(userId, teamId, new UpdateModelCredentialCommand { Id = id, DisplayName = "Renamed", BaseUrl = shown }); + + var row = await LoadAsync(id); + row.BaseUrl.ShouldBe(stored, "the masked round-trip keeps the real embedded credentials — the platform still needs them to call the gateway"); + row.DisplayName.ShouldBe("Renamed"); + (await DecryptKeyAsync(id)).ShouldBe("sk-embedded-0123456789"); + (await SendAsync(userId, teamId, new ListModelCredentialsQuery())).ShouldHaveSingleItem().BaseUrl.ShouldNotContain("CANARY"); + } + + [Fact] + public async Task A_masked_placeholder_left_in_an_edited_url_is_refused_and_changes_nothing() + { + const string stored = "https://svc:CANARY-MC-PW-03@gw.example/v1"; + var (userId, teamId) = await SeedTeamAsync(); + var id = await SendAsync(userId, teamId, new AddModelCredentialCommand { Provider = "Custom", DisplayName = "Embedded", ApiKey = "sk-embedded-0123456789", BaseUrl = stored }); + + await Should.ThrowAsync(() => SendAsync(userId, teamId, new UpdateModelCredentialCommand { Id = id, DisplayName = "Embedded", BaseUrl = "https://***:***@gw.example/v2" })); + + (await LoadAsync(id)).BaseUrl.ShouldBe(stored, "the placeholder is never stored as the credential"); + } + [Fact] public async Task Revoke_soft_deletes_clears_the_key_and_drops_from_the_list() { @@ -221,6 +289,12 @@ private async Task SendAsync(Guid userId, Guid teamId, IReques return await scope.Resolve().Send(request); } + private async Task LoadAsync(Guid id) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().ModelCredential.AsNoTracking().SingleAsync(c => c.Id == id); + } + private async Task DecryptKeyAsync(Guid id) { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/ProviderInstanceEndpointBindingFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/ProviderInstanceEndpointBindingFlowTests.cs new file mode 100644 index 000000000..afc1c22c8 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/ProviderInstanceEndpointBindingFlowTests.cs @@ -0,0 +1,182 @@ +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Commands.ProviderInstances; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using CodeSpace.Messages.Queries.ProviderInstances; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.ProviderInstances; + +/// +/// A git provider connection's secrets — the tokens members connected through it and its OAuth client secret — are bound +/// to the host they were entered for. An edit that moves the host while a token is connected is refused and changes +/// nothing; one that moves it past a stored client secret needs the secret again; anything that keeps the host moves +/// freely. The URLs the list shows are masked where they embed credentials, and sending a masked URL back keeps the +/// stored one — real credentials included — so the edit form and any client that PATCHes back what it GETs keep working. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class ProviderInstanceEndpointBindingFlowTests +{ + private const string Host = "https://gitlab.acme.test"; + + private readonly PostgresFixture _fixture; + + public ProviderInstanceEndpointBindingFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Theory] + [InlineData("base", CredentialStatus.Active)] + [InlineData("api", CredentialStatus.Active)] + [InlineData("base", CredentialStatus.Expired)] + [InlineData("api", CredentialStatus.Error)] + public async Task A_host_move_is_refused_while_a_token_is_connected_and_changes_nothing(string field, CredentialStatus status) + { + var (teamId, instanceId) = await SeedInstanceAsync(Host, apiUrl: null, withOAuth: false); + await SeedCredentialAsync(teamId, instanceId, status); + + var refusal = await Should.ThrowAsync(() => UpdateAsync(teamId, new UpdateProviderInstanceCommand { ProviderInstanceId = instanceId, DisplayName = "renamed", BaseUrl = field == "base" ? "https://attacker.test" : null, ApiUrl = field == "api" ? "https://attacker.test" : null })); + + refusal.BoundCredentialCount.ShouldBe(1, "an Expired or Error token is still decrypted and presented, so it binds the host too"); + var row = await LoadAsync(instanceId); + row.BaseUrl.ShouldBe(Host); + row.ApiUrl.ShouldBeNull(); + row.DisplayName.ShouldBe("GitLab", "a refused edit changes nothing, the rest of it included"); + } + + [Fact] + public async Task A_disconnected_token_does_not_bind_the_host() + { + var (teamId, instanceId) = await SeedInstanceAsync(Host, apiUrl: null, withOAuth: false); + await SeedCredentialAsync(teamId, instanceId, CredentialStatus.Revoked); + + await UpdateAsync(teamId, new UpdateProviderInstanceCommand { ProviderInstanceId = instanceId, BaseUrl = "https://gitlab.new.test" }); + + (await LoadAsync(instanceId)).BaseUrl.ShouldBe("https://gitlab.new.test", "a disconnect revoked the row and blanked its token: nothing would follow the move"); + } + + [Fact] + public async Task A_path_only_edit_moves_nothing_and_is_kept_with_a_token_connected() + { + var (teamId, instanceId) = await SeedInstanceAsync(Host, apiUrl: null, withOAuth: true); + await SeedCredentialAsync(teamId, instanceId, CredentialStatus.Active); + + await UpdateAsync(teamId, new UpdateProviderInstanceCommand { ProviderInstanceId = instanceId, BaseUrl = Host + "/gitlab/", ApiUrl = Host + "/gitlab/api" }); + + var row = await LoadAsync(instanceId); + row.BaseUrl.ShouldBe(Host + "/gitlab"); + row.ApiUrl.ShouldBe(Host + "/gitlab/api"); + } + + [Fact] + public async Task A_host_move_past_a_stored_client_secret_needs_the_secret_again() + { + var (teamId, instanceId) = await SeedInstanceAsync(Host, apiUrl: null, withOAuth: true); + + await Should.ThrowAsync(() => UpdateAsync(teamId, new UpdateProviderInstanceCommand { ProviderInstanceId = instanceId, BaseUrl = "https://gitlab.new.test", OauthClientSecret = "" })); + + (await LoadAsync(instanceId)).BaseUrl.ShouldBe(Host); + + await UpdateAsync(teamId, new UpdateProviderInstanceCommand { ProviderInstanceId = instanceId, BaseUrl = "https://gitlab.new.test", OauthClientId = "new-host-app", OauthClientSecret = "new-host-secret" }); + + using var verify = _fixture.BeginScope(); + var row = await verify.Resolve().ProviderInstance.AsNoTracking().SingleAsync(p => p.Id == instanceId); + row.BaseUrl.ShouldBe("https://gitlab.new.test"); + verify.Resolve().Decrypt(row.OauthClientSecretEnc!).ShouldBe("new-host-secret"); + } + + [Fact] + public async Task The_list_masks_embedded_credentials_and_sending_them_back_keeps_the_stored_urls() + { + var canary = "CANARY" + Guid.NewGuid().ToString("N")[..12]; + var baseUrl = $"https://svc:{canary}@gitlab.acme.test"; + var apiUrl = $"https://gitlab.acme.test/api?private_token={canary}"; + var webUrl = $"https://viewer:{canary}@gitlab.acme.test"; + var (teamId, instanceId) = await SeedInstanceAsync(baseUrl, apiUrl, withOAuth: false, webUrl); + await SeedCredentialAsync(teamId, instanceId, CredentialStatus.Active); + + var listed = (await ListAsync(teamId)).Single(p => p.Id == instanceId); + + listed.BaseUrl.ShouldBe("https://***:***@gitlab.acme.test"); + listed.ApiUrl.ShouldBe("https://gitlab.acme.test/api?private_token=***"); + listed.WebUrl.ShouldBe("https://***:***@gitlab.acme.test"); + + await UpdateAsync(teamId, new UpdateProviderInstanceCommand { ProviderInstanceId = instanceId, DisplayName = "renamed", BaseUrl = listed.BaseUrl, ApiUrl = listed.ApiUrl, WebUrl = listed.WebUrl }); + + var row = await LoadAsync(instanceId); + row.DisplayName.ShouldBe("renamed"); + row.BaseUrl.ShouldBe(baseUrl, "the masked form sent back means unchanged — the real credentials are kept"); + row.ApiUrl.ShouldBe(apiUrl); + row.WebUrl.ShouldBe(webUrl); + } + + [Fact] + public async Task A_placeholder_left_in_an_edited_url_is_refused_rather_than_stored_as_the_credential() + { + var baseUrl = "https://svc:CANARY-PW-0123@gitlab.acme.test"; + var (teamId, instanceId) = await SeedInstanceAsync(baseUrl, apiUrl: null, withOAuth: false); + + var refusal = await Should.ThrowAsync(() => UpdateAsync(teamId, new UpdateProviderInstanceCommand { ProviderInstanceId = instanceId, BaseUrl = "https://***:***@gitlab.acme.test/gitlab" })); + + refusal.Message.ShouldNotContain("CANARY"); + (await LoadAsync(instanceId)).BaseUrl.ShouldBe(baseUrl); + } + + private async Task UpdateAsync(Guid teamId, UpdateProviderInstanceCommand command) + { + using var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin); + + await scope.Resolve().Send(command); + } + + private async Task> ListAsync(Guid teamId) + { + using var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin); + + return await scope.Resolve().Send(new ListProviderInstancesQuery()); + } + + private async Task LoadAsync(Guid instanceId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().ProviderInstance.AsNoTracking().SingleAsync(p => p.Id == instanceId); + } + + private async Task<(Guid TeamId, Guid InstanceId)> SeedInstanceAsync(string baseUrl, string? apiUrl, bool withOAuth, string? webUrl = null) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var encryptor = scope.Resolve(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + var owner = new User { Id = Guid.NewGuid(), Email = $"owner-{suffix}@x", Name = "Owner" }; + var team = new Team { Id = Guid.NewGuid(), Slug = $"team-{suffix}", Name = "Team" }; + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.GitLab, DisplayName = "GitLab", BaseUrl = baseUrl, ApiUrl = apiUrl, WebUrl = webUrl, OauthClientId = withOAuth ? "client-id" : null, OauthClientSecretEnc = withOAuth ? encryptor.Encrypt("original-secret") : null }; + + db.User.Add(owner); + db.Team.Add(team); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = team.Id, UserId = owner.Id, Role = TeamRole.Owner }); + db.Project.Add(TestProjectSeed.BuildDefaultProject(team.Id, owner.Id)); + db.ProviderInstance.Add(instance); + await db.SaveChangesAsync(); + + return (team.Id, instance.Id); + } + + private async Task SeedCredentialAsync(Guid teamId, Guid instanceId, CredentialStatus status) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var payload = status == CredentialStatus.Revoked ? "" : scope.Resolve().Encrypt(scope.Resolve().Serialize(new PatPayload { Token = "glpat-BOUND-" + Guid.NewGuid().ToString("N") })); + + db.Credential.Add(new Credential { Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instanceId, AuthType = AuthType.Pat, DisplayName = "member pat", EncryptedPayload = payload, Status = status }); + await db.SaveChangesAsync(); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookRegistrationCredentialEchoTests.cs b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookRegistrationCredentialEchoTests.cs new file mode 100644 index 000000000..bd31bb31c --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Webhooks/WebhookRegistrationCredentialEchoTests.cs @@ -0,0 +1,154 @@ +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Webhooks.Registration; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Shouldly; +using static CodeSpace.IntegrationTests.Webhooks.StubProviderHost; + +namespace CodeSpace.IntegrationTests.Webhooks; + +/// +/// A webhook registration the provider refuses while QUOTING the credential it was presented must not persist that +/// credential. The refusal lands in repository_webhook.last_error, *_webhook_attempt.error and +/// *_webhook_attempt.response_body, which GET /api/repositories/{id}/webhooks (and the connection coverage +/// read) return to any member of the team, a Viewer included. +/// +/// Driven through the real registrars and the real GitLab and GitHub providers — NGitLab, Octokit and the raw +/// group-hook calls — against a loopback provider that echoes the request's PRIVATE-TOKEN and +/// Authorization into every refusal. The fixture check that the token WAS presented is what makes the absence an +/// end-to-end claim rather than a claim about a provider that never echoes. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class WebhookRegistrationCredentialEchoTests +{ + private readonly PostgresFixture _fixture; + + public WebhookRegistrationCredentialEchoTests(PostgresFixture fixture) { _fixture = fixture; } + + public enum Refused { Create, List } + + [Theory] + [InlineData(ProviderKind.GitLab, Refused.Create)] + [InlineData(ProviderKind.GitLab, Refused.List)] + [InlineData(ProviderKind.GitHub, Refused.Create)] + public async Task A_refused_repository_hook_persists_no_echoed_token(ProviderKind kind, Refused refused) + { + var token = (kind == ProviderKind.GitLab ? "glpat-" : "ghp_") + "HOOKECHO" + Guid.NewGuid().ToString("N"); + var hooks = kind == ProviderKind.GitLab ? "/api/v4/projects/4242/hooks" : "/repositories/4242/hooks"; + using var provider = new StubProviderHost() + .Answer("POST", hooks, request => Echo(403, request)) + .Answer("GET", hooks, request => refused == Refused.List || kind == ProviderKind.GitHub ? Echo(403, request) : new StubReply(200, "[]")) + .Answer("GET", "/api/v4/projects/4242", 200, GitLabProjectJson) + .Answer("GET", "/repositories/4242", 200, GitHubRepositoryJson); + + var webhookId = await SeedEnqueuedRepositoryWebhookAsync(kind, provider.BaseUrl, token); + + await RunAsync(registrar => registrar.RunAsync(webhookId, CancellationToken.None)); + + using var verify = _fixture.BeginScope(); + var db = verify.Resolve(); + var hook = await db.RepositoryWebhook.AsNoTracking().SingleAsync(w => w.Id == webhookId); + var attempt = await db.RepositoryWebhookAttempt.AsNoTracking().SingleAsync(a => a.RepositoryWebhookId == webhookId); + + PresentedTheToken(provider, token); + hook.RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Failed, "fixture check: the provider refused the registration"); + hook.LastError.ShouldNotBeNullOrEmpty(); + hook.LastError.ShouldNotContain(token, Case.Sensitive, "last_error is returned to every member"); + attempt.Error.ShouldNotBeNullOrEmpty(); + attempt.Error.ShouldNotContain(token, Case.Sensitive, "the attempt's error is returned to every member"); + (attempt.ResponseBody ?? "").ShouldNotContain(token, Case.Sensitive, "the attempt's response body is returned to every member"); + (attempt.RequestHeadersJson ?? "").ShouldNotContain(token, Case.Sensitive); + (attempt.ResponseBody ?? attempt.Error).ShouldContain("403", Case.Sensitive, "the provider's refusal is still the evidence"); + } + + [Fact] + public async Task A_refused_group_hook_persists_no_echoed_token() + { + var token = "glpat-GROUPECHO" + Guid.NewGuid().ToString("N"); + using var provider = new StubProviderHost() + .Answer("POST", "/api/v4/groups/acme%2Fplatform/hooks", request => Echo(403, request)) + .Answer("GET", "/api/v4/groups/acme%2Fplatform/hooks", 200, "[]"); + + var hookId = await SeedEnqueuedGitLabGroupHookAsync(provider.BaseUrl, token); + + await RunAsync(registrar => registrar.RunAsync(hookId, CancellationToken.None)); + + using var verify = _fixture.BeginScope(); + var db = verify.Resolve(); + var hook = await db.ConnectionWebhook.AsNoTracking().SingleAsync(w => w.Id == hookId); + var attempt = await db.ConnectionWebhookAttempt.AsNoTracking().SingleAsync(a => a.ConnectionWebhookId == hookId); + + PresentedTheToken(provider, token); + hook.RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Failed, "fixture check: GitLab refused the group hook"); + (hook.LastError ?? "").ShouldNotContain(token, Case.Sensitive); + (attempt.Error ?? "").ShouldNotContain(token, Case.Sensitive); + attempt.StatusCode.ShouldBe(403); + attempt.ResponseBody.ShouldNotBeNull().ShouldContain("403 refused", Case.Sensitive, "GitLab's own words are the evidence the attempt row exists to keep"); + attempt.ResponseBody.ShouldNotContain(token, Case.Sensitive, "the attempt's response body is returned to every member"); + } + + private static void PresentedTheToken(StubProviderHost provider, string token) => + provider.Requests.ShouldContain(request => (request.AuthorizationHeader ?? "").Contains(token) || (request.PrivateTokenHeader ?? "").Contains(token), "fixture check: the token was presented, so every refusal quoted it"); + + private static StubReply Echo(int status, RecordedRequest request) => new(status, JsonSerializer.Serialize(new { message = $"{status} refused (PRIVATE-TOKEN={request.PrivateTokenHeader}; Authorization={request.AuthorizationHeader})" })); + + /// A registrar swallows the failure it records; anything it lets escape is not this test's subject, so the rows are what is read. + private async Task RunAsync(Func run) where TRegistrar : notnull + { + using var scope = _fixture.BeginScope(); + + try { await run(scope.Resolve()); } + catch (Exception) { /* recorded on the row before it escaped, if it did */ } + } + + private async Task SeedEnqueuedRepositoryWebhookAsync(ProviderKind kind, string baseUrl, string token) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var encryptor = scope.Resolve(); + var team = new Team { Id = Guid.NewGuid(), Slug = $"t-{Guid.NewGuid().ToString("N")[..8]}", Name = "Team" }; + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = kind, DisplayName = "echoing provider", BaseUrl = baseUrl, ApiUrl = baseUrl }; + var credential = new Credential { Id = Guid.NewGuid(), TeamId = team.Id, ProviderInstanceId = instance.Id, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.Pat, DisplayName = "connection", EncryptedPayload = encryptor.Encrypt(scope.Resolve().Serialize(new PatPayload { Token = token })), Status = CredentialStatus.Active }; + var repository = new Repository { Id = Guid.NewGuid(), TeamId = team.Id, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ExternalId = "4242", NamespacePath = "acme", Name = "api", FullPath = "acme/api", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "https://forge.test/acme/api", Status = RepositoryStatus.Active }; + var webhookId = Guid.NewGuid(); + + db.Team.Add(team); + db.ProviderInstance.Add(instance); + db.Credential.Add(credential); + db.Repository.Add(repository); + db.RepositoryWebhook.Add(new RepositoryWebhook { Id = webhookId, RepositoryId = repository.Id, CallbackUrl = $"https://codespace.test/api/webhooks/{webhookId}", SecretEnc = encryptor.Encrypt("whsec-echo-loopback"), SubscribedEvents = ["push"], RegistrationStatus = RepositoryWebhookRegistrationStatus.Enqueued, EnqueuedAt = DateTimeOffset.UtcNow }); + await db.SaveChangesAsync(); + + return webhookId; + } + + private async Task SeedEnqueuedGitLabGroupHookAsync(string baseUrl, string token) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var encryptor = scope.Resolve(); + var team = new Team { Id = Guid.NewGuid(), Slug = $"t-{Guid.NewGuid().ToString("N")[..8]}", Name = "Team" }; + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.GitLab, DisplayName = "echoing gitlab", BaseUrl = baseUrl, ApiUrl = baseUrl, WebhookScope = ProviderWebhookScope.Connection }; + var credential = new Credential { Id = Guid.NewGuid(), TeamId = team.Id, ProviderInstanceId = instance.Id, AuthType = AuthType.Pat, DisplayName = "connection", EncryptedPayload = encryptor.Encrypt(scope.Resolve().Serialize(new PatPayload { Token = token })) }; + var hookId = Guid.NewGuid(); + + db.Team.Add(team); + db.ProviderInstance.Add(instance); + db.Credential.Add(credential); + db.ConnectionWebhook.Add(new ConnectionWebhook { Id = hookId, ProviderInstanceId = instance.Id, CredentialId = credential.Id, OwnerPath = "acme/platform", CallbackUrl = $"https://codespace.test/api/webhooks/connection/{hookId}", SecretEnc = encryptor.Encrypt("whsec-echo-loopback"), SubscribedEvents = ["Push Hook"], RegistrationStatus = RepositoryWebhookRegistrationStatus.Enqueued }); + await db.SaveChangesAsync(); + + return hookId; + } + + private const string GitLabProjectJson = """{"id":4242,"name":"api","path":"api","path_with_namespace":"acme/api","namespace":{"id":1,"full_path":"acme","path":"acme","name":"acme","kind":"group"},"default_branch":"main","visibility":"private","web_url":"https://forge.test/acme/api","http_url_to_repo":"https://forge.test/acme/api.git","ssh_url_to_repo":"git@forge.test:acme/api.git","archived":false}"""; + + private const string GitHubRepositoryJson = """{"id":4242,"name":"api","full_name":"acme/api","owner":{"login":"acme"},"default_branch":"main","private":true,"html_url":"https://forge.test/acme/api","clone_url":"https://forge.test/acme/api.git","ssh_url":"git@forge.test:acme/api.git","archived":false}"""; +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/LlmGatewayKeyEchoFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/LlmGatewayKeyEchoFlowTests.cs new file mode 100644 index 000000000..27f22558c --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/LlmGatewayKeyEchoFlowTests.cs @@ -0,0 +1,161 @@ +using System.Net; +using System.Text; +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Workflows.Engine; +using CodeSpace.Core.Services.Workflows.Llm; +using CodeSpace.Core.Services.Workflows.Llm.OpenAi; +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 MediatR; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// A model gateway that ECHOES the key it rejected (Invalid API key provided: sk-…) must not get that key into +/// anything the run persists. Drives the real engine, the real over real HTTP to a loopback +/// gateway, and real Postgres; then scans every row the run wrote — node, interaction and external-call failure records, +/// the park marker, the run row, every table — for the key. A terminal 401 (the node fails) and a 429 (the node parks, +/// holding the fault text in its wait), and the production named-client pipeline +/// () beside the fixture's. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class LlmGatewayKeyEchoFlowTests +{ + private readonly PostgresFixture _fixture; + + public LlmGatewayKeyEchoFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Theory] + [InlineData(HttpStatusCode.Unauthorized, false, WorkflowRunRecordTypes.NodeFailed)] + [InlineData(HttpStatusCode.TooManyRequests, false, WorkflowRunRecordTypes.NodeSuspended)] + [InlineData(HttpStatusCode.Unauthorized, true, WorkflowRunRecordTypes.NodeFailed)] + public async Task A_gateway_echoing_the_key_leaves_no_key_in_any_failure_record(HttpStatusCode status, bool productionPipeline, string nodeOutcome) + { + var key = $"sk-ECHO-FLOW-{Guid.NewGuid():N}"; + await using var gateway = LoopbackResponder.Start(head => EchoTheKey(head, status)); + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture, inProcessPool: false); + await SeedGatewayCredentialAsync(teamId, key, $"http://127.0.0.1:{gateway.Port}/v1"); + + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, await CreateLlmWorkflowAsync(teamId, userId), teamId); + await RunEngineAsync(runId, productionPipeline); + + gateway.Requests.ShouldContain(r => r.Contains(key), "fixture check: the gateway was presented the key, so its echo carried it"); + var records = await RecordsAsync(runId); + records.Select(r => r.RecordType).ShouldContain(nodeOutcome, $"fixture check: a {(int)status} reaches the node's {nodeOutcome} path"); + records.Select(r => r.RecordType).ShouldContain(WorkflowRunRecordTypes.InteractionFailed); + records.Select(r => r.RecordType).ShouldContain(WorkflowRunRecordTypes.ExternalCallFailed); + + records.Where(r => r.PayloadJson.Contains(key)).Select(r => r.RecordType).ShouldBeEmpty("no record may carry the echoed key"); + records.Single(r => r.RecordType == WorkflowRunRecordTypes.InteractionFailed).PayloadJson.ShouldContain("Invalid API key provided: ***", Case.Sensitive, "the gateway's own words survive around the struck key"); + (await RowsContainingAsync(key)).ShouldBeEmpty("no table may hold the echoed key in plaintext — the park marker and the run row included"); + } + + private static string EchoTheKey(string head, HttpStatusCode status) + { + var presented = head.Split("\r\n").FirstOrDefault(line => line.StartsWith("Authorization:", StringComparison.OrdinalIgnoreCase))?.Split(' ').LastOrDefault() ?? ""; + var body = JsonSerializer.Serialize(new { error = new { message = $"Invalid API key provided: {presented}", type = "invalid_request_error" } }); + + return $"HTTP/1.1 {(int)status} {status}\r\nContent-Type: application/json\r\nRetry-After: 0\r\nContent-Length: {Encoding.UTF8.GetByteCount(body)}\r\nConnection: close\r\n\r\n{body}"; + } + + private async Task SeedGatewayCredentialAsync(Guid teamId, string key, string baseUrl) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var credentialId = Guid.NewGuid(); + + db.ModelCredential.Add(new ModelCredential { Id = credentialId, TeamId = teamId, Provider = "OpenAI", DisplayName = "echoing gateway", EncryptedApiKey = scope.Resolve().Encrypt(key), BaseUrl = baseUrl, Status = CredentialStatus.Active, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }); + db.ModelCredentialModel.Add(new ModelCredentialModel { Id = Guid.NewGuid(), ModelCredentialId = credentialId, ModelId = "gpt-echo", Source = ModelSource.Manual, Enabled = true }); + await db.SaveChangesAsync(); + } + + private async Task CreateLlmWorkflowAsync(Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); + + return await scope.Resolve().Send(new CreateWorkflowCommand + { + Name = $"echo-{Guid.NewGuid():N}"[..20], Description = null, Activations = new List(), Enabled = true, + Definition = new WorkflowDefinition + { + SchemaVersion = 1, + Nodes = new List + { + new() { Id = "start", TypeKey = "trigger.manual", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "llm", TypeKey = "llm.complete", Config = WorkflowsTestSeed.Json("""{ "provider": "OpenAI", "model": "gpt-echo" }"""), Inputs = WorkflowsTestSeed.Json("""{ "userPrompt": "hello" }""") }, + new() { Id = "end", TypeKey = "builtin.terminal", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + }, + Edges = new List { new() { From = "start", To = "llm" }, new() { From = "llm", To = "end" } }, + }, + }); + } + + /// The fixture's registered OpenAI client, or — for the production pipeline — the real over (accounting handler + resilience), wrapped in the same recording decorator production uses. + private async Task RunEngineAsync(Guid runId, bool productionPipeline) + { + if (!productionPipeline) + { + using var fixtureScope = _fixture.BeginScope(); + await fixtureScope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + return; + } + + var collection = new ServiceCollection(); + collection.AddLogging(); + collection.AddLlmHttpClients(); + await using var services = collection.BuildServiceProvider(); + var registry = new LLMClientRegistry([new RecordingStreamingStructuredLLMClientDecorator(new OpenAiClient(services.GetRequiredService()))]); + + using var scope = _fixture.BeginScope(builder => builder.RegisterInstance(registry).As()); + await scope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + } + + private async Task> RecordsAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().WorkflowRunRecord.AsNoTracking().Where(r => r.RunId == runId).OrderBy(r => r.Sequence).ToListAsync(); + } + + /// Every base table, every row, as text — the key must be nowhere, not only in the record types this test names. + private async Task> RowsContainingAsync(string needle) + { + var hits = new List(); + await using var connection = new NpgsqlConnection(_fixture.ConnectionString); + await connection.OpenAsync(); + + foreach (var table in await BaseTablesAsync(connection)) + { + await using var query = new NpgsqlCommand($"SELECT count(*) FROM {table} x WHERE x::text LIKE @needle", connection); + query.Parameters.AddWithValue("needle", $"%{needle}%"); + + if ((long)(await query.ExecuteScalarAsync())! > 0) hits.Add(table); + } + + return hits; + } + + private static async Task> BaseTablesAsync(NpgsqlConnection connection) + { + var tables = new List(); + await using var list = new NpgsqlCommand("SELECT quote_ident(table_schema) || '.' || quote_ident(table_name) FROM information_schema.tables WHERE table_type = 'BASE TABLE' AND table_schema NOT IN ('pg_catalog', 'information_schema')", connection); + await using var reader = await list.ExecuteReaderAsync(); + + while (await reader.ReadAsync()) tables.Add(reader.GetString(0)); + + return tables; + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialEndpointBindingTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialEndpointBindingTests.cs new file mode 100644 index 000000000..92f3de1a3 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Agents/ModelCredentialEndpointBindingTests.cs @@ -0,0 +1,69 @@ +using CodeSpace.Core.Services.Agents.ModelCredentials; +using CodeSpace.Messages.Exceptions; +using CodeSpace.Messages.Failures; +using Shouldly; + +namespace CodeSpace.UnitTests.Agents; + +/// +/// Pins the two rules that bind a model credential's stored key to the endpoint it was entered for: the base URL an +/// edit submits (the masked display form sent back means "unchanged"), and when an endpoint change needs the key again. +/// The DB round-trips are integration-pinned in ModelCredentialFlowTests. +/// +[Trait("Category", "Unit")] +public class ModelCredentialEndpointBindingTests +{ + private const string Stored = "https://svc:CANARY-PW-11@gw.example/v1?api-key=CANARY-Q-22&api-version=2024-01-01"; + + [Fact] + public void The_masked_display_form_sent_back_keeps_the_stored_url() => + ModelCredentialService.SubmittedBaseUrl(Stored, "https://***:***@gw.example/v1?api-key=***&api-version=2024-01-01").ShouldBe(Stored); + + [Theory] + [InlineData(Stored, Stored)] + [InlineData("https://gw.example/v2", "https://gw.example/v2")] + [InlineData("https://new:CANARY-NEW@gw.example/v1", "https://new:CANARY-NEW@gw.example/v1")] + [InlineData("", null)] + [InlineData(" ", null)] + [InlineData(null, null)] + public void Any_other_value_is_taken_as_submitted(string? submitted, string? expected) => ModelCredentialService.SubmittedBaseUrl(Stored, submitted).ShouldBe(expected); + + [Fact] + public void A_url_with_nothing_to_mask_round_trips_as_itself() => ModelCredentialService.SubmittedBaseUrl("http://localhost:11434", "http://localhost:11434").ShouldBe("http://localhost:11434"); + + [Theory] + [InlineData("https://***:***@gw.example/v2?api-key=***&api-version=2024-01-01")] + [InlineData("https://***:***@evil.example/v1?api-key=***&api-version=2024-01-01")] + [InlineData("https://svc:***@gw.example/v1")] + public void A_masked_placeholder_left_in_an_edited_url_is_refused_rather_than_stored_as_the_credential(string submitted) + { + var refusal = Should.Throw(() => ModelCredentialService.SubmittedBaseUrl(Stored, submitted)); + + refusal.Message.ShouldContain("***"); + refusal.Message.ShouldNotContain("CANARY"); + } + + [Theory] + [InlineData(true, "https://gw.example/v1", "https://evil.example/v1", null, true)] + [InlineData(true, "https://gw.example/v1", "http://gw.example/v1", "", true)] + [InlineData(true, "https://gw.example/v1", "https://gw.example:8443/v1", " ", true)] + [InlineData(true, null, "https://evil.example/v1", null, true)] + [InlineData(true, "https://gw.example/v1", null, null, true)] + [InlineData(true, "https://gw.example/v1", "https://gw.example/v2", null, false)] + [InlineData(true, "https://gw.example/v1", "https://GW.example:443/v1", null, false)] + [InlineData(true, "https://gw.example/v1", "https://evil.example/v1", "sk-new-key-0123", false)] + [InlineData(false, "https://gw.example/v1", "https://evil.example/v1", null, false)] + public void An_endpoint_move_needs_the_key_again_only_when_a_stored_key_would_follow_it(bool hasStoredKey, string? storedUrl, string? nextUrl, string? apiKey, bool expected) => + ModelCredentialService.KeyMustBeReentered(hasStoredKey, storedUrl, nextUrl, apiKey).ShouldBe(expected); + + [Fact] + public void The_refusal_is_a_typed_invalid_request_with_a_pinned_code() + { + var id = Guid.NewGuid(); + var refusal = new ModelEndpointKeyRequiredException(id); + + ((IFailure)refusal).Kind.ShouldBe(FailureKind.Invalid); + ((IFailure)refusal).Code.ShouldBe("model_endpoint_key_required"); + ((IFailure)refusal).Details!["credentialId"].ShouldBe(id); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs index 486152a69..1b4f06526 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs @@ -75,6 +75,9 @@ public void The_wire_codes_are_pinned() FailureCodes.ProviderError.ShouldBe("provider_error"); FailureCodes.RateLimited.ShouldBe("rate_limited"); FailureCodes.CredentialDisconnected.ShouldBe("credential_disconnected"); + FailureCodes.ModelEndpointKeyRequired.ShouldBe("model_endpoint_key_required"); + FailureCodes.ProviderEndpointCredentialsBound.ShouldBe("provider_endpoint_credentials_bound"); + FailureCodes.ProviderEndpointSecretRequired.ShouldBe("provider_endpoint_secret_required"); FailureCodes.WorkflowDefinitionInvalid.ShouldBe("workflow_definition_invalid"); FailureCodes.TaskRouteConfirmationRequired.ShouldBe("task_route_confirmation_required"); FailureCodes.WorkspaceUnresolvable.ShouldBe("workspace_unresolvable"); diff --git a/backend/tests/CodeSpace.UnitTests/Providers/ProviderInstanceEndpointBindingTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/ProviderInstanceEndpointBindingTests.cs new file mode 100644 index 000000000..0eb347809 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/ProviderInstanceEndpointBindingTests.cs @@ -0,0 +1,61 @@ +using CodeSpace.Core.Services.ProviderInstances; +using CodeSpace.Messages.Exceptions; +using CodeSpace.Messages.Failures; +using Shouldly; + +namespace CodeSpace.UnitTests.Providers; + +/// +/// Pins when an edit MOVES a provider connection's endpoint — where its bound tokens and OAuth client secret are sent — +/// and the typed refusals that keep them from following a repoint. A connection presents its credentials at two places: +/// the API (ApiUrl, or the base URL when unset) for every provider call, and the base URL itself for the OAuth +/// token, authorize and revoke endpoints. Moving either one's scheme, host or port is a move; a path or a credential +/// slot is not. The DB round-trips are integration-pinned in ProviderInstanceEndpointBindingFlowTests. +/// +[Trait("Category", "Unit")] +public class ProviderInstanceEndpointBindingTests +{ + [Theory] + [InlineData("https://gitlab.acme.test", null, "https://gitlab.acme.test", null, false)] + [InlineData("https://gitlab.acme.test", null, "https://GITLAB.acme.test:443", null, false)] + [InlineData("https://gitlab.acme.test", null, "https://gitlab.acme.test/gitlab", null, false)] + [InlineData("https://gitlab.acme.test", "https://gitlab.acme.test/api", "https://gitlab.acme.test", "https://gitlab.acme.test/other", false)] + [InlineData("https://gitlab.acme.test", null, "https://gitlab.acme.test", "https://gitlab.acme.test/api", false)] + [InlineData("https://gitlab.acme.test", "https://gitlab.acme.test", "https://gitlab.acme.test", null, false)] + [InlineData("https://svc:pw@gitlab.acme.test", null, "https://gitlab.acme.test", null, false)] + [InlineData("https://gitlab.acme.test", null, "https://attacker.test", null, true)] + [InlineData("https://gitlab.acme.test", null, "http://gitlab.acme.test", null, true)] + [InlineData("https://gitlab.acme.test", null, "https://gitlab.acme.test:8443", null, true)] + [InlineData("https://gitlab.acme.test", null, "https://gitlab.acme.test", "https://attacker.test", true)] + [InlineData("https://gitlab.acme.test", "https://api.acme.test", "https://gitlab.acme.test", null, true)] + [InlineData("https://gitlab.acme.test", "https://api.acme.test", "https://attacker.test", "https://api.acme.test", true)] + // Conservative on purpose: the rule reads URLs, not each provider's derivation, so it cannot know github.com's API + // already lives at api.github.com — spelling it out reads as a move, refused only while a token or client secret is bound. + [InlineData("https://github.com", null, "https://github.com", "https://api.github.com", true)] + public void An_edit_moves_the_endpoint_when_the_api_or_the_base_url_changes_scheme_host_or_port(string storedBaseUrl, string? storedApiUrl, string nextBaseUrl, string? nextApiUrl, bool expected) => + ProviderInstanceService.EndpointMoves(storedBaseUrl, storedApiUrl, nextBaseUrl, nextApiUrl).ShouldBe(expected); + + [Fact] + public void Bound_credentials_refuse_the_move_as_a_precondition_naming_how_many() + { + var id = Guid.NewGuid(); + var refusal = new ProviderEndpointCredentialsBoundException(id, 3); + + ((IFailure)refusal).Kind.ShouldBe(FailureKind.PreconditionRequired); + ((IFailure)refusal).Code.ShouldBe("provider_endpoint_credentials_bound"); + ((IFailure)refusal).Details!["providerInstanceId"].ShouldBe(id); + ((IFailure)refusal).Details!["boundCredentialCount"].ShouldBe(3); + refusal.Message.ShouldContain("3"); + } + + [Fact] + public void A_stored_oauth_secret_refuses_the_move_until_it_is_entered_again() + { + var id = Guid.NewGuid(); + var refusal = new ProviderEndpointSecretRequiredException(id); + + ((IFailure)refusal).Kind.ShouldBe(FailureKind.Invalid); + ((IFailure)refusal).Code.ShouldBe("provider_endpoint_secret_required"); + ((IFailure)refusal).Details!["providerInstanceId"].ShouldBe(id); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/ProviderRefusalEchoTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/ProviderRefusalEchoTests.cs new file mode 100644 index 000000000..86f4fa720 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/ProviderRefusalEchoTests.cs @@ -0,0 +1,218 @@ +using System.Text.Json; +using CodeSpace.Core.Failures; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Providers; +using CodeSpace.Core.Services.Providers.Auth; +using CodeSpace.Core.Services.Providers.Errors; +using CodeSpace.Core.Services.Providers.Events; +using CodeSpace.Core.Services.Providers.GitHub; +using CodeSpace.Core.Services.Providers.GitLab; +using CodeSpace.Core.Services.Providers.Resilience; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using CodeSpace.Messages.Failures; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using static CodeSpace.IntegrationTests.Webhooks.StubProviderHost; + +namespace CodeSpace.UnitTests.Providers; + +/// +/// The provider calls that read a refusal's body THEMSELVES — GitLab's raw /unapprove, the project and group hook +/// creates, the hook-list probes, GitHub's hook creates — must strike the token they presented before that body becomes +/// an exception message or a webhook-attempt diagnostic. Those land in external_call.failed, +/// repository_webhook.last_error and *_webhook_attempt.response_body, which every member of the team can +/// read; a provider (or a proxy in front of it) that quotes the PRIVATE-TOKEN / Authorization it rejected +/// would otherwise hand the token to all of them. The resilience translation never sees these bodies, so it cannot be +/// the one to strike them. +/// +/// Driven through the real providers, the real auth resolver and the real resilience layer against a loopback +/// provider that echoes the presented credential headers into every refusal. +/// +[Trait("Category", "Unit")] +public sealed class ProviderRefusalEchoTests : IDisposable +{ + private const string GitLabToken = "glpat-REFUSAL-ECHO-CANARY-1a2b3c4d"; + private const string GitHubToken = "ghp_REFUSALECHOCANARY5e6f7a8b9c"; + + private readonly StubProviderHost _forge = new(); + + public void Dispose() => _forge.Dispose(); + + [Theory] + [InlineData(401)] + [InlineData(403)] + [InlineData(500)] + public async Task A_refused_unapprove_never_carries_the_presented_token(int status) + { + _forge.Answer("POST", "/merge_requests/7/unapprove", request => Echo(status, request)); + + var refusal = await Should.ThrowAsync(() => GitLab().SubmitReviewAsync(Context(ProviderKind.GitLab, GitLabToken), Repository, 7, RequestChanges, CancellationToken.None)); + + PresentedTheToken(GitLabToken); + refusal.StatusCode.ShouldBe(status); + refusal.ProviderMessage.ShouldContain($"{status} refused", Case.Sensitive, "GitLab's own words stay legible around the struck token"); + ShouldCarryNoToken(refusal, GitLabToken); + } + + [Fact] + public async Task A_scope_refused_unapprove_never_carries_the_presented_token_in_its_hint() + { + _forge.Answer("POST", "/merge_requests/7/unapprove", request => new StubReply(403, JsonSerializer.Serialize(new { error = "insufficient_scope", scope = "api", error_description = $"refused PRIVATE-TOKEN={request.PrivateTokenHeader}" }))); + + var refusal = await Should.ThrowAsync(() => GitLab().SubmitReviewAsync(Context(ProviderKind.GitLab, GitLabToken), Repository, 7, RequestChanges, CancellationToken.None)); + + PresentedTheToken(GitLabToken); + refusal.MissingScopes.ShouldBe(["api"], "the scope is still read off the provider's answer"); + refusal.ProviderHint.ShouldNotBeNull().ShouldNotContain(GitLabToken); + ShouldCarryNoToken(refusal, GitLabToken); + } + + [Fact] + public async Task A_refused_gitlab_project_hook_create_never_carries_the_presented_token() + { + _forge.Answer("GET", "/api/v4/projects/4242/hooks", 200, "[]").Answer("POST", "/api/v4/projects/4242/hooks", request => Echo(403, request)); + + var refusal = await Should.ThrowAsync(() => GitLab().RegisterWebhookAsync(Context(ProviderKind.GitLab, GitLabToken), Repository, Hook, CancellationToken.None)); + + PresentedTheToken(GitLabToken); + refusal.Message.ShouldContain("403", Case.Sensitive, "the status the provider answered stays in the error"); + ShouldCarryNoToken(refusal, GitLabToken); + } + + [Fact] + public async Task A_refused_gitlab_project_hook_list_never_carries_the_presented_token() + { + _forge.Answer("GET", "/api/v4/projects/4242/hooks", request => Echo(403, request)); + + var refusal = await Should.ThrowAsync(() => GitLab().FindWebhookByCallbackUrlAsync(Context(ProviderKind.GitLab, GitLabToken), Repository, Hook.CallbackUrl, CancellationToken.None)); + + PresentedTheToken(GitLabToken); + refusal.Diagnostic.StatusCode.ShouldBe(403); + refusal.Diagnostic.ResponseBody.ShouldNotBeNull().ShouldContain("403 refused", Case.Sensitive, "GitLab's answer is still the evidence"); + ShouldCarryNoToken(refusal, GitLabToken); + } + + [Theory] + [InlineData(false, 403)] + [InlineData(false, 404)] + [InlineData(true, 401)] + public async Task A_refused_gitlab_group_hook_call_never_carries_the_presented_token(bool list, int status) + { + _forge.Answer(list ? "GET" : "POST", "/api/v4/groups/acme%2Fplatform/hooks", request => Echo(status, request)).Answer("GET", "/api/v4/groups/acme%2Fplatform/hooks", 200, "[]"); + + var context = Context(ProviderKind.GitLab, GitLabToken); + Task Call() => list ? GitLab().FindConnectionWebhookByCallbackUrlAsync(context, "acme/platform", Hook.CallbackUrl, CancellationToken.None) : (Task)GitLab().RegisterConnectionWebhookAsync(context, "acme/platform", Hook, CancellationToken.None); + var refusal = await Should.ThrowAsync(Call); + + PresentedTheToken(GitLabToken); + var evidence = Diagnosed(refusal).ShouldNotBeNull("a group-hook refusal carries its evidence down the chain"); + evidence.Diagnostic.StatusCode.ShouldBe(status); + evidence.Diagnostic.ResponseBody.ShouldNotBeNull().ShouldContain($"{status} refused", Case.Sensitive, "GitLab's own words are the evidence for the refusal"); + ShouldCarryNoToken(refusal, GitLabToken); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_refused_github_hook_create_never_carries_the_presented_token(bool organization) + { + var hooks = organization ? "/orgs/acme/hooks" : "/repositories/4242/hooks"; + _forge.Answer("POST", hooks, request => Echo(403, request)).Answer("GET", hooks, request => Echo(403, request)); + + var context = Context(ProviderKind.GitHub, GitHubToken); + var refusal = await Should.ThrowAsync(() => organization ? GitHub().RegisterConnectionWebhookAsync(context, "acme", Hook, CancellationToken.None) : GitHub().RegisterWebhookAsync(context, Repository, Hook, CancellationToken.None)); + + PresentedTheToken(GitHubToken); + refusal.Diagnostic.StatusCode.ShouldBe(403); + refusal.Diagnostic.ResponseBody.ShouldNotBeNull().ShouldContain("403 refused", Case.Sensitive, "GitHub's answer is still the evidence"); + ShouldCarryNoToken(refusal, GitHubToken); + } + + /// Fixture check: the provider really was presented the token, so its refusal really quoted it. + private void PresentedTheToken(string token) => + _forge.Requests.ShouldContain(request => (request.AuthorizationHeader ?? "").Contains(token) || (request.PrivateTokenHeader ?? "").Contains(token), "fixture check: the token was presented, so the refusal echoed it"); + + /// + /// Every surface the failure reaches a reader through: the thrown message (what last_error, the attempt's + /// error and external_call.failed persist), each typed failure down the chain, the classified client + /// message and details, and the webhook-attempt diagnostic. The SDK's own exception stays the innermost one, raw, for + /// the operator log — it is never rendered to a reader, so it is not a surface here. + /// + private static void ShouldCarryNoToken(Exception refusal, string token) + { + refusal.Message.ShouldNotContain(token, Case.Sensitive, $"{refusal.GetType().Name}.Message"); + + for (var current = refusal.InnerException; current != null; current = current.InnerException) + { + if (current is IFailure) current.Message.ShouldNotContain(token, Case.Sensitive, $"{current.GetType().Name}.Message"); + } + + var classified = FailureClassifier.Classify(refusal); + classified.ClientMessage.ShouldNotContain(token, Case.Sensitive); + (classified.Details?.Values ?? []).ShouldAllBe(value => value == null || !value.ToString()!.Contains(token)); + + if (Diagnosed(refusal) is not { } evidence) return; + + evidence.Diagnostic.ResponseBody?.ShouldNotContain(token, Case.Sensitive); + evidence.Diagnostic.Request?.Url.ShouldNotContain(token, Case.Sensitive); + JsonSerializer.Serialize(evidence.Diagnostic.Request?.Headers).ShouldNotContain(token, Case.Sensitive); + } + + private static ProviderWebhookRegistrationException? Diagnosed(Exception refusal) + { + for (var current = refusal; current != null; current = current.InnerException) + { + if (current is ProviderWebhookRegistrationException diagnosed) return diagnosed; + } + + return null; + } + + private static StubReply Echo(int status, RecordedRequest request) => new(status, JsonSerializer.Serialize(new { message = $"{status} refused (PRIVATE-TOKEN={request.PrivateTokenHeader}; Authorization={request.AuthorizationHeader})" })); + + private ProviderContext Context(ProviderKind kind, string token) + { + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = kind, DisplayName = "loopback", BaseUrl = _forge.BaseUrl, ApiUrl = kind == ProviderKind.GitHub ? _forge.BaseUrl : null }; + var credential = new Credential { Id = Guid.NewGuid(), TeamId = instance.TeamId, ProviderInstanceId = instance.Id, AuthType = AuthType.Pat, DisplayName = "pat", EncryptedPayload = "unused", Status = CredentialStatus.Active }; + + _tokens[kind] = token; + + return new ProviderContext(instance, credential); + } + + private readonly Dictionary _tokens = new(); + + private ProviderAuthResolver Resolver() => new([new FixedToken(ProviderKind.GitLab, () => _tokens[ProviderKind.GitLab]), new FixedToken(ProviderKind.GitHub, () => _tokens[ProviderKind.GitHub])]); + + private GitLabRepositoryProvider GitLab() + { + var resilience = new ExternalCallResilience(new ProviderErrorMapperRegistry(new IProviderErrorMapper[] { new GitLabErrorMapper() }), NullLogger.Instance); + + return new GitLabRepositoryProvider(Resolver(), resilience, new GitLabSignatureVerifier(), new GitLabEventNormalizer(new ProviderEventSubscriptionRegistry(Array.Empty())), new GitLabWebhookRepositoryIdentifier()); + } + + private GitHubRepositoryProvider GitHub() + { + var resilience = new ExternalCallResilience(new ProviderErrorMapperRegistry(new IProviderErrorMapper[] { new GitHubErrorMapper() }), NullLogger.Instance); + + return new GitHubRepositoryProvider(Resolver(), resilience, new GitHubSignatureVerifier(), new GitHubEventNormalizer(new ProviderEventSubscriptionRegistry(Array.Empty())), new GitHubWebhookRepositoryIdentifier()); + } + + private static readonly SubmitPullRequestReviewInput RequestChanges = new() { Verdict = PullRequestReviewVerdict.RequestChanges, Body = "needs work" }; + + private static readonly WebhookRegistration Hook = new() { CallbackUrl = "https://codespace.test/api/webhooks/0f5e2c7a-9b1d-4e3f-8a6b-2c4d6e8f0a1b", Secret = "whsec-loopback-0000", SubscribedEvents = ["push"] }; + + private static readonly RemoteRepository Repository = new() { ExternalId = "4242", NamespacePath = "acme", Name = "api", FullPath = "acme/api", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "https://forge.test/acme/api" }; + + private sealed class FixedToken(ProviderKind kind, Func token) : IProviderAuthStrategy + { + public ProviderKind Kind => kind; + + public AuthType AuthType => AuthType.Pat; + + public Task ResolveAsync(ProviderContext context, CancellationToken cancellationToken) => Task.FromResult(new ResolvedAuth { Token = token() }); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/Resilience/ProviderErrorEchoTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/Resilience/ProviderErrorEchoTests.cs new file mode 100644 index 000000000..c2995863a --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/Resilience/ProviderErrorEchoTests.cs @@ -0,0 +1,134 @@ +using System.Net; +using CodeSpace.Core.Failures; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Providers; +using CodeSpace.Core.Services.Providers.Auth; +using CodeSpace.Core.Services.Providers.Errors; +using CodeSpace.Core.Services.Providers.GitLab; +using CodeSpace.Core.Services.Providers.Resilience; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; + +namespace CodeSpace.UnitTests.Providers.Resilience; + +/// +/// A git provider (or a proxy in front of it) that ECHOES the token it was presented must never get that token into the +/// typed failure a caller — a Viewer on a repository page, a git.* node's error — is shown. The SDK-to- +/// translation strikes every token the auth resolver handed out for the instance, +/// and drops the SDK's "Original call: <url>" suffix from the message the client reads. +/// +[Trait("Category", "Unit")] +public class ProviderErrorEchoTests +{ + private const string Token = "glpat-ECHO-CANARY-5e6f7a8b9c"; + + public enum Sdk { DuckTyped, GitLab, GitHub } + + [Theory] + [InlineData(Sdk.DuckTyped)] + [InlineData(Sdk.GitLab)] + [InlineData(Sdk.GitHub)] + public async Task An_echoed_token_never_reaches_the_translated_failure(Sdk sdk) + { + var (instance, resilience) = await ResolvedInstanceAsync(ProviderKind.GitLab, Token); + + var ex = await Should.ThrowAsync(() => resilience.ExecuteAsync(instance, "ListBranchesAsync", _ => throw Echo(sdk, HttpStatusCode.NotFound), CancellationToken.None)); + + ex.StatusCode.ShouldBe(404); + ShouldCarryNoToken(ex.Message); + ShouldCarryNoToken(ex.ProviderMessage); + ShouldCarryNoToken(FailureClassifier.Classify(ex).ClientMessage); + ex.ProviderMessage.ShouldContain("Project Not Found", Case.Sensitive, "the provider's own words stay legible around the struck token"); + } + + [Fact] + public async Task An_echoed_token_never_reaches_an_insufficient_scope_hint() + { + var (instance, resilience) = await ResolvedInstanceAsync(ProviderKind.GitLab, Token, new GitLabErrorMapper()); + var echo = new NGitLab.GitLabException($"GitLab server returned an error (Forbidden): insufficient_scope (PRIVATE-TOKEN={Token}). Original call: Post http://127.0.0.1/api/v4/projects/42/approve") { StatusCode = HttpStatusCode.Forbidden }; + + var ex = await Should.ThrowAsync(() => resilience.ExecuteAsync(instance, "SubmitReviewAsync", _ => throw echo, CancellationToken.None)); + + ex.MissingScopes.ShouldBe(["api"], "the scope classification reads the raw SDK failure and is unchanged"); + ShouldCarryNoToken(ex.ProviderHint); + ShouldCarryNoToken(FailureClassifier.Classify(ex).Details!["providerHint"] as string); + } + + [Fact] + public async Task An_instance_whose_token_was_never_resolved_keeps_the_providers_words_without_the_original_call() + { + var resilience = Resilience(); + var instance = Instance(ProviderKind.GitHub); + + var ex = await Should.ThrowAsync(() => resilience.ExecuteAsync(instance, "GetStatsAsync", _ => throw new Octokit.ApiException("Not Found. Original call: Get https://api.github.com/repos/acme/web", HttpStatusCode.NotFound), CancellationToken.None)); + + ex.ProviderMessage.ShouldBe("Not Found."); + } + + [Fact] + public async Task A_token_resolved_for_one_instance_object_does_not_mask_another() + { + var (_, resilience) = await ResolvedInstanceAsync(ProviderKind.GitLab, Token); + var other = Instance(ProviderKind.GitLab); + + var ex = await Should.ThrowAsync(() => resilience.ExecuteAsync(other, "ListBranchesAsync", _ => throw Echo(Sdk.DuckTyped, HttpStatusCode.NotFound), CancellationToken.None)); + + ex.ProviderMessage.ShouldContain(Token, Case.Sensitive, "fixture check: nothing was resolved for this instance, so nothing is struck — the rule is per presentation, not a global word list"); + } + + private static void ShouldCarryNoToken(string? text) + { + text.ShouldNotBeNull(); + text.ShouldNotContain(Token); + text.ShouldNotContain("Original call"); + } + + /// The real auth resolver hands the instance its token — the one step every provider call takes before it reaches the resilience layer. + private static async Task<(ProviderInstance Instance, ExternalCallResilience Resilience)> ResolvedInstanceAsync(ProviderKind kind, string token, IProviderErrorMapper? mapper = null) + { + var instance = Instance(kind); + var credential = new Credential { Id = Guid.NewGuid(), TeamId = instance.TeamId, ProviderInstanceId = instance.Id, AuthType = AuthType.Pat, DisplayName = "pat", EncryptedPayload = "", Status = CredentialStatus.Active }; + var resolver = new ProviderAuthResolver([new FixedTokenStrategy(kind, token)]); + + (await resolver.ResolveAsync(new ProviderContext(instance, credential), CancellationToken.None)).Token.ShouldBe(token, "fixture check: the resolver handed out the token"); + + return (instance, Resilience(mapper)); + } + + private static ExternalCallResilience Resilience(IProviderErrorMapper? mapper = null) => new(new SingleMapperRegistry(mapper), NullLogger.Instance); + + private static ProviderInstance Instance(ProviderKind kind) => new() { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = kind, DisplayName = "echo", BaseUrl = "http://127.0.0.1" }; + + private static Exception Echo(Sdk sdk, HttpStatusCode status) + { + var message = $"GitLab server returned an error ({status}): 404 Project Not Found (PRIVATE-TOKEN=; Authorization=Bearer {Token}). Original call: Get http://127.0.0.1/api/v4/projects/42/repository/branches?private_token={Token}"; + + return sdk switch + { + Sdk.GitLab => new NGitLab.GitLabException(message) { StatusCode = status }, + Sdk.GitHub => new Octokit.ApiException(message, status), + _ => new DuckTypedSdkException(message, (int)status), + }; + } + + private sealed class DuckTypedSdkException(string message, int statusCode) : Exception(message) + { + public int StatusCode { get; } = statusCode; + } + + private sealed class FixedTokenStrategy(ProviderKind kind, string token) : IProviderAuthStrategy + { + public ProviderKind Kind => kind; + + public AuthType AuthType => AuthType.Pat; + + public Task ResolveAsync(ProviderContext context, CancellationToken cancellationToken) => Task.FromResult(new ResolvedAuth { Token = token }); + } + + private sealed class SingleMapperRegistry(IProviderErrorMapper? mapper) : IProviderErrorMapperRegistry + { + public IProviderErrorMapper? Get(ProviderKind kind) => mapper?.Kind == kind ? mapper : null; + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Redaction/CredentialNeedlesTests.cs b/backend/tests/CodeSpace.UnitTests/Redaction/CredentialNeedlesTests.cs new file mode 100644 index 000000000..1939f9771 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Redaction/CredentialNeedlesTests.cs @@ -0,0 +1,123 @@ +using System.Net.Http.Headers; +using CodeSpace.Core.Services.Agents; +using CodeSpace.Core.Services.Redaction; +using CodeSpace.Messages.Agents; +using Shouldly; + +namespace CodeSpace.UnitTests.Redaction; + +/// +/// Pins the shared needle rule: what a credential, a URL and an outgoing request PRESENT, so every echo scrub (the agent +/// run's redactor, the in-process model transport, the git-provider error translation) strikes the same values. +/// +[Trait("Category", "Unit")] +public class CredentialNeedlesTests +{ + private const string Key = "sk-NEEDLE-0f9e8d7c6b5a"; + + [Fact] + public void MinimumLength_is_pinned() => CredentialNeedles.MinimumLength.ShouldBe(8); + + [Theory] + [InlineData("API_KEY", true)] + [InlineData("api-key", true)] + [InlineData("x-api-key", true)] + [InlineData("Authorization", true)] + [InlineData("PRIVATE-TOKEN", true)] + [InlineData("access_token", true)] + [InlineData("api-version", false)] + [InlineData("anthropic-version", false)] + [InlineData("model", false)] + public void MarksASecret_reads_the_name(string name, bool expected) => CredentialNeedles.MarksASecret(name).ShouldBe(expected); + + [Fact] + public void FromUrl_yields_userinfo_and_secret_query_values_in_both_spellings() + { + var needles = CredentialNeedles.FromUrl("https://svc:p%40ss-word-123@gw.example/v1?api-key=q%2Bkey-456789&api-version=2024-01-01").ToList(); + + needles.ShouldContain("svc"); + needles.ShouldContain("p%40ss-word-123"); + needles.ShouldContain("p@ss-word-123"); + needles.ShouldContain("q%2Bkey-456789"); + needles.ShouldContain("q+key-456789"); + needles.ShouldNotContain("2024-01-01", "a non-secret query value stays readable"); + needles.ShouldNotContain(n => n.Contains("gw.example"), "the host is what says which endpoint answered"); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not a url")] + [InlineData("https://gw.example/v1")] + public void FromUrl_yields_nothing_when_the_url_carries_no_credential(string? url) => CredentialNeedles.FromUrl(url).ShouldBeEmpty(); + + [Fact] + public void FromCredential_yields_the_key_then_the_url_parts() + { + var needles = CredentialNeedles.FromCredential(new ResolvedModelCredential { Provider = "OpenAI", ApiKey = Key, BaseUrl = "https://gw.example/v1?token=tok-123456789" }).ToList(); + + needles[0].ShouldBe(Key); + needles.ShouldContain("tok-123456789"); + } + + [Fact] + public void FromRequest_yields_the_bearer_credential_not_the_scheme() + { + var request = new HttpRequestMessage(HttpMethod.Post, "https://gw.example/v1/chat/completions"); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", Key); + + var needles = CredentialNeedles.FromRequest(request).ToList(); + + needles.ShouldContain(Key); + needles.ShouldNotContain("Bearer"); + } + + [Theory] + [InlineData("x-api-key")] + [InlineData("api-key")] + [InlineData("PRIVATE-TOKEN")] + public void FromRequest_yields_a_secret_named_header_value(string header) + { + var request = new HttpRequestMessage(HttpMethod.Post, "https://gw.example/v1/messages"); + request.Headers.Add(header, Key); + request.Headers.Add("anthropic-version", "2023-06-01"); + + var needles = CredentialNeedles.FromRequest(request).ToList(); + + needles.ShouldContain(Key); + needles.ShouldNotContain("2023-06-01", "a version header is not a credential"); + } + + [Fact] + public void FromRequest_yields_the_credential_parts_of_the_request_url() + { + var request = new HttpRequestMessage(HttpMethod.Post, "https://svc:pw-0123456789@gw.example/v1/chat/completions?api-key=q-0123456789"); + + var needles = CredentialNeedles.FromRequest(request).ToList(); + + needles.ShouldContain("pw-0123456789"); + needles.ShouldContain("q-0123456789"); + } + + [Fact] + public void Redactor_drops_fragments_below_the_minimum_length() + { + var redactor = CredentialNeedles.Redactor(["EMPTY", null, Key]); + + redactor.Redact($"EMPTY key {Key} rejected").ShouldBe($"EMPTY key {SecretRedactor.Placeholder} rejected"); + } + + [Fact] + public void Redactor_with_no_usable_needle_is_the_identity() => CredentialNeedles.Redactor(["short", null]).ShouldBeSameAs(SecretRedactor.None); + + [Theory] + [InlineData("{\"message\":\"401 Unauthorized (PRIVATE-TOKEN=sk-NEEDLE-0f9e8d7c6b5a)\"}", "{\"message\":\"401 Unauthorized (PRIVATE-TOKEN=***)\"}")] + [InlineData("Bearer sk-NEEDLE-0f9e8d7c6b5a, again sk-NEEDLE-0f9e8d7c6b5a", "Bearer ***, again ***")] + [InlineData("403 Forbidden", "403 Forbidden")] + [InlineData("", "")] + [InlineData(null, null)] + public void Strike_removes_every_presented_credential_from_an_answer_and_keeps_the_rest(string? answer, string? expected) => CredentialNeedles.Strike(answer, [Key]).ShouldBe(expected); + + [Fact] + public void Strike_with_nothing_presented_is_the_identity() => CredentialNeedles.Strike("401 (PRIVATE-TOKEN=short)", [null, "short"]).ShouldBe("401 (PRIVATE-TOKEN=short)"); +} diff --git a/backend/tests/CodeSpace.UnitTests/Redaction/CredentialUrlTests.cs b/backend/tests/CodeSpace.UnitTests/Redaction/CredentialUrlTests.cs new file mode 100644 index 000000000..aed6c008e --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Redaction/CredentialUrlTests.cs @@ -0,0 +1,98 @@ +using CodeSpace.Core.Services.Redaction; +using Shouldly; + +namespace CodeSpace.UnitTests.Redaction; + +/// +/// Pins the display form of a credential-bearing URL (userinfo and secret-named query values masked, everything else +/// verbatim) and the endpoint identity a stored key is bound to (scheme, host, port). +/// +[Trait("Category", "Unit")] +public class CredentialUrlTests +{ + [Fact] + public void Mask_is_pinned() => CredentialUrl.Mask.ShouldBe("***"); + + [Theory] + [InlineData(null, null)] + [InlineData("", "")] + [InlineData("https://gw.example/v1", "https://gw.example/v1")] + [InlineData("https://GW.example:443/v1/", "https://GW.example:443/v1/")] + [InlineData("http://localhost:11434", "http://localhost:11434")] + [InlineData("https://usr:CANARY-PW@gw.example/v1", "https://***:***@gw.example/v1")] + [InlineData("https://CANARY-TOKEN@gw.example/v1", "https://***@gw.example/v1")] + [InlineData("https://gw.example/v1?api-key=CANARY-Q&api-version=2024-01-01", "https://gw.example/v1?api-key=***&api-version=2024-01-01")] + [InlineData("https://gw.example/v1?key=CANARY-Q#frag", "https://gw.example/v1?key=***#frag")] + [InlineData("https://gw.example?api%2Dkey=CANARY-Q", "https://gw.example?api%2Dkey=***")] + [InlineData("https://gw.example/v1?token=", "https://gw.example/v1?token=")] + [InlineData("https://u:CANARY-PW@gw.example:8443/v1?model=m&access_token=CANARY-Q", "https://***:***@gw.example:8443/v1?model=m&access_token=***")] + [InlineData("https://gw.example/v1/CANARY-PATH/chat", "https://gw.example/v1/CANARY-PATH/chat")] + public void Masked_hides_userinfo_and_secret_query_values_and_keeps_the_rest_verbatim(string? stored, string? expected) => CredentialUrl.Masked(stored).ShouldBe(expected); + + [Fact] + public void Masked_never_contains_the_embedded_credentials() + { + var masked = CredentialUrl.Masked("https://usr:CANARY-PW-1@gw.example/v1?api-key=CANARY-Q-2&secret=CANARY-Q-3"); + + masked.ShouldNotContain("CANARY"); + masked.ShouldContain("gw.example/v1"); + } + + [Theory] + [InlineData("https://***:***@gw.example/v1", true)] + [InlineData("https://usr:***@gw.example/v1", true)] + [InlineData("https://gw.example/v2?api-key=***", true)] + [InlineData("https://gw.example/v1?note=***", false)] + [InlineData("https://usr:pw@gw.example/v1?api-key=abc", false)] + [InlineData("https://gw.example/v1", false)] + [InlineData(null, false)] + public void CarriesMask_finds_the_placeholder_in_a_credential_slot(string? url, bool expected) => CredentialUrl.CarriesMask(url).ShouldBe(expected); + + [Theory] + [InlineData("https://gw.example/v1", "https://gw.example/v2", true)] + [InlineData("https://gw.example/v1", "https://GW.EXAMPLE/v1", true)] + [InlineData("https://gw.example/v1", "https://gw.example:443/v1", true)] + [InlineData("https://a:b@gw.example/v1", "https://c:d@gw.example/v1?api-key=x", true)] + [InlineData(null, null, true)] + [InlineData("", null, true)] + [InlineData("https://gw.example/v1", "https://evil.example/v1", false)] + [InlineData("https://gw.example/v1", "http://gw.example/v1", false)] + [InlineData("https://gw.example/v1", "https://gw.example:8443/v1", false)] + [InlineData(null, "https://gw.example/v1", false)] + [InlineData("https://gw.example/v1", null, false)] + [InlineData("gw.example:4000", "gw.example:5000", false)] + [InlineData("gw.example:4000", "gw.example:4000", true)] + public void SameEndpoint_compares_scheme_host_and_port(string? stored, string? next, bool expected) => CredentialUrl.SameEndpoint(stored, next).ShouldBe(expected); + + private const string StoredWithCredentials = "https://svc:CANARY-PW-11@gw.example/v1?api-key=CANARY-Q-22&api-version=2024-01-01"; + + [Theory] + [InlineData(StoredWithCredentials, "https://***:***@gw.example/v1?api-key=***&api-version=2024-01-01", StoredWithCredentials)] + [InlineData(StoredWithCredentials, StoredWithCredentials, StoredWithCredentials)] + [InlineData(StoredWithCredentials, "https://gw.example/v2", "https://gw.example/v2")] + [InlineData(StoredWithCredentials, "https://new:CANARY-NEW@gw.example/v1", "https://new:CANARY-NEW@gw.example/v1")] + [InlineData("http://localhost:11434", "http://localhost:11434", "http://localhost:11434")] + [InlineData(null, "https://gw.example/v1", "https://gw.example/v1")] + [InlineData("https://gw.example/v1", "", "")] + [InlineData("https://gw.example/v1", null, null)] + public void KeepStoredIfMasked_reads_the_display_form_sent_back_as_unchanged(string? stored, string? submitted, string? expected) => CredentialUrl.KeepStoredIfMasked(stored, submitted).ShouldBe(expected); + + [Theory] + [InlineData("https://***:***@gw.example/v2?api-key=***&api-version=2024-01-01")] + [InlineData("https://***:***@evil.example/v1?api-key=***&api-version=2024-01-01")] + [InlineData("https://svc:***@gw.example/v1")] + public void KeepStoredIfMasked_refuses_a_placeholder_left_in_an_edited_url(string submitted) + { + var refusal = Should.Throw(() => CredentialUrl.KeepStoredIfMasked(StoredWithCredentials, submitted)); + + refusal.Message.ShouldContain(CredentialUrl.Mask); + refusal.Message.ShouldNotContain("CANARY"); + } + + [Theory] + [InlineData(null, "(provider default)")] + [InlineData("https://usr:CANARY@gw.example/v1?api-key=CANARY", "https://gw.example:443")] + [InlineData("http://10.0.0.5:4000/v1", "http://10.0.0.5:4000")] + [InlineData("not a url CANARY", "(unparseable)")] + public void Endpoint_names_only_scheme_host_and_port(string? url, string expected) => CredentialUrl.Endpoint(url).ShouldBe(expected); +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/Llm/RecordingLLMClientDecoratorTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/Llm/RecordingLLMClientDecoratorTests.cs index f5afd9713..250fd3923 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/Llm/RecordingLLMClientDecoratorTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/Llm/RecordingLLMClientDecoratorTests.cs @@ -7,6 +7,7 @@ using CodeSpace.Core.Services.Workflows.Runtime; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.RunData; +using CodeSpace.Messages.Agents; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Contracts; using Microsoft.Extensions.Time.Testing; @@ -124,6 +125,69 @@ public async Task An_inner_throw_records_started_then_failed_and_rethrows_verbat logger.Calls[1].Payload.GetProperty("error").GetString().ShouldBe("gateway boom"); } + // ── Backstop: a failure that reached the decorator still quoting the call's own credential ── + // The transport strikes an echoed key before the typed failure exists; a failure raised anywhere else (a client that + // bypasses the transport, a handler that wraps the response) is struck here, against the credential the call carried. + + private const string EchoedKey = "sk-BACKSTOP-CANARY-4a5b6c7d"; + + private static ResolvedModelCredential EchoedCredential => new() { Provider = "OpenAI", ApiKey = EchoedKey, BaseUrl = "https://svc:pw-BACKSTOP-CANARY@gw.example/v1" }; + + [Fact] + public async Task A_structured_failure_quoting_the_calls_credential_is_recorded_without_it_and_rethrown_verbatim() + { + var fault = new InvalidOperationException($"upstream rejected {EchoedKey} at https://svc:pw-BACKSTOP-CANARY@gw.example/v1"); + var logger = new CapturingLogger(); + + using (PushScope(logger)) + { + var thrown = await Should.ThrowAsync(() => new RecordingStructuredLLMClientDecorator(new ThrowingClient(fault)).CompleteStructuredAsync(Request() with { Credential = EchoedCredential }, CancellationToken.None)); + thrown.ShouldBeSameAs(fault, "the backstop strikes the RECORD, never the exception the caller handles"); + } + + ShouldRecordNoCredential(logger); + } + + [Fact] + public async Task A_plain_failure_quoting_the_calls_credential_is_recorded_without_it() + { + var logger = new CapturingLogger(); + + using (PushScope(logger)) + { + await Should.ThrowAsync(() => new RecordingLLMClientDecorator(new ThrowingClient(new InvalidOperationException($"bad key {EchoedKey}"))).CompleteAsync(new LLMCompletionRequest { Model = "m", SystemPrompt = "s", UserPrompt = "u", Credential = EchoedCredential }, CancellationToken.None)); + } + + ShouldRecordNoCredential(logger); + } + + [Fact] + public async Task A_streamed_failure_quoting_the_calls_credential_is_recorded_without_it() + { + var inner = new StreamingFakeClient { ThrowAfter = 1, ThrowWith = new InvalidOperationException($"stream cut, key {EchoedKey}") }; + var logger = new CapturingLogger(); + + using (PushScope(logger)) + { + await Should.ThrowAsync(async () => + { + await foreach (var _ in new RecordingStreamingStructuredLLMClientDecorator(inner).StreamAsync(new LLMCompletionRequest { Model = "m", SystemPrompt = "s", UserPrompt = "u", Credential = EchoedCredential }, CancellationToken.None)) { } + }); + } + + ShouldRecordNoCredential(logger); + } + + private static void ShouldRecordNoCredential(CapturingLogger logger) + { + var failed = logger.Calls.Single(c => c.RecordType == WorkflowRunRecordTypes.InteractionFailed).Payload.GetProperty("error").GetString(); + + failed.ShouldNotBeNull(); + failed.ShouldNotContain(EchoedKey); + failed.ShouldNotContain("pw-BACKSTOP-CANARY"); + failed.ShouldContain(PersistenceSecretRedactor.Marker); + } + [Fact] public async Task A_large_prompt_and_completion_offload_to_an_artifact_ref_not_inline() { diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/LlmTransportCredentialEchoTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/LlmTransportCredentialEchoTests.cs new file mode 100644 index 000000000..b923eac80 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/LlmTransportCredentialEchoTests.cs @@ -0,0 +1,127 @@ +using System.Net; +using System.Text; +using System.Text.Json; +using CodeSpace.Core.Services.Workflows.Llm; +using CodeSpace.Core.Services.Workflows.Llm.Anthropic; +using CodeSpace.Core.Services.Workflows.Llm.OpenAi; +using CodeSpace.Messages.Agents; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// A gateway that ECHOES the credential it was presented — a 401 quoting the key it rejected, a 429/5xx from a proxy that +/// reflects request headers and URL — must never get that credential into the typed failure. The failure's message is +/// what the engine writes to node / interaction / external-call failure records and what a Viewer reads on the run, so +/// the transport strikes every value the request presented before the exception exists. Both wires, buffered and +/// streamed; the classification (status, category, Retry-After) is read from the raw body and is unchanged. +/// +[Trait("Category", "Unit")] +public class LlmTransportCredentialEchoTests +{ + private const string Key = "sk-ECHO-CANARY-7c1d0e9f8a2b"; + + public enum Wire { OpenAi, Anthropic } + + [Theory] + [InlineData(Wire.OpenAi, HttpStatusCode.Unauthorized, LlmErrorCategory.AuthFailed)] + [InlineData(Wire.OpenAi, HttpStatusCode.TooManyRequests, LlmErrorCategory.RateLimited)] + [InlineData(Wire.OpenAi, HttpStatusCode.InternalServerError, LlmErrorCategory.Transient)] + [InlineData(Wire.Anthropic, HttpStatusCode.Unauthorized, LlmErrorCategory.AuthFailed)] + [InlineData(Wire.Anthropic, HttpStatusCode.TooManyRequests, LlmErrorCategory.RateLimited)] + [InlineData(Wire.Anthropic, HttpStatusCode.BadGateway, LlmErrorCategory.Transient)] + public async Task A_buffered_call_never_carries_an_echoed_key_into_the_failure(Wire wire, HttpStatusCode status, LlmErrorCategory expected) + { + var handler = new EchoingHandler(status); + + var ex = await Should.ThrowAsync(() => Client(wire, handler).CompleteAsync(Request(wire, "https://gw.example/v1"), CancellationToken.None)); + + handler.Echoed.ShouldContain(Key, Case.Sensitive, "fixture check: the gateway really echoed the presented key"); + ShouldCarryNoKey(ex, (int)status, expected); + } + + [Theory] + [InlineData(Wire.OpenAi, HttpStatusCode.Unauthorized, LlmErrorCategory.AuthFailed)] + [InlineData(Wire.OpenAi, HttpStatusCode.TooManyRequests, LlmErrorCategory.RateLimited)] + [InlineData(Wire.OpenAi, HttpStatusCode.ServiceUnavailable, LlmErrorCategory.Transient)] + [InlineData(Wire.Anthropic, HttpStatusCode.Forbidden, LlmErrorCategory.AuthFailed)] + [InlineData(Wire.Anthropic, HttpStatusCode.TooManyRequests, LlmErrorCategory.RateLimited)] + [InlineData(Wire.Anthropic, HttpStatusCode.InternalServerError, LlmErrorCategory.Transient)] + public async Task A_streamed_call_never_carries_an_echoed_key_into_the_failure(Wire wire, HttpStatusCode status, LlmErrorCategory expected) + { + var handler = new EchoingHandler(status); + var streaming = (IStreamingLLMClient)Client(wire, handler); + + var ex = await Should.ThrowAsync(async () => + { + await foreach (var _ in streaming.StreamAsync(Request(wire, "https://gw.example/v1"), CancellationToken.None)) { } + }); + + handler.Echoed.ShouldContain(Key, Case.Sensitive, "fixture check: the gateway really echoed the presented key"); + ShouldCarryNoKey(ex, (int)status, expected); + } + + [Fact] + public async Task Credentials_embedded_in_the_base_url_are_struck_from_an_echo_of_the_request_url() + { + const string password = "pw-ECHO-CANARY-2f3e4d5c"; + var handler = new EchoingHandler(HttpStatusCode.Unauthorized); + + var ex = await Should.ThrowAsync(() => Client(Wire.Anthropic, handler).CompleteAsync(Request(Wire.Anthropic, $"https://svc:{password}@gw.example"), CancellationToken.None)); + + handler.Echoed.ShouldContain(password, Case.Sensitive, "fixture check: the echoed request URL carried the userinfo"); + ex.Message.ShouldNotContain(password); + ex.ProviderMessage.ShouldNotContain(password); + ex.Message.ShouldContain("gw.example", Case.Sensitive, "the host stays readable — it says which endpoint answered"); + } + + [Fact] + public async Task A_body_with_no_echo_reaches_the_failure_verbatim() + { + var handler = new EchoingHandler(HttpStatusCode.Unauthorized) { EchoCredential = false }; + + var ex = await Should.ThrowAsync(() => Client(Wire.OpenAi, handler).CompleteAsync(Request(Wire.OpenAi, "https://gw.example/v1"), CancellationToken.None)); + + ex.ProviderMessage.ShouldBe(handler.Echoed, "nothing to strike — the operator reads the gateway's own words"); + } + + private static void ShouldCarryNoKey(LlmApiException ex, int status, LlmErrorCategory expected) + { + ex.Message.ShouldNotContain(Key); + ex.ProviderMessage.ShouldNotContain(Key); + ex.ProviderMessage.ShouldContain("***", Case.Sensitive, "the echo is struck in place, so the rest of the gateway's sentence stays legible"); + ex.StatusCode.ShouldBe(status); + ex.Category.ShouldBe(expected, "the category is read from the raw body — striking the echo cannot change it"); + } + + private static ILLMClient Client(Wire wire, HttpMessageHandler handler) => wire == Wire.OpenAi ? new OpenAiClient(new StubFactory(handler)) : new AnthropicClient(new StubFactory(handler)); + + private static LLMCompletionRequest Request(Wire wire, string baseUrl) => new() + { + Model = "m", SystemPrompt = "s", UserPrompt = "u", + Credential = new ResolvedModelCredential { Provider = wire == Wire.OpenAi ? "OpenAI" : "Anthropic", ApiKey = Key, BaseUrl = baseUrl }, + }; + + private sealed class StubFactory(HttpMessageHandler handler) : IHttpClientFactory + { + public HttpClient CreateClient(string name) => new(handler, disposeHandler: false); + } + + /// Answers every request with and a JSON error quoting the request's credential headers and URL — what a reflecting proxy or a careless gateway sends. + private sealed class EchoingHandler(HttpStatusCode status) : HttpMessageHandler + { + public bool EchoCredential { get; init; } = true; + + public string Echoed { get; private set; } = ""; + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + var presented = request.Headers.Authorization?.ToString() ?? string.Join(",", request.Headers.TryGetValues("x-api-key", out var values) ? values : []); + var message = EchoCredential ? $"Invalid API key provided: {presented} (request {request.RequestUri!.OriginalString})" : "Invalid API key provided."; + + Echoed = JsonSerializer.Serialize(new { error = new { message, type = "invalid_request_error" } }); + + return Task.FromResult(new HttpResponseMessage(status) { Content = new StringContent(Echoed, Encoding.UTF8, "application/json") }); + } + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/NodeObservabilityTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/NodeObservabilityTests.cs index 510d9595a..dd0b634a3 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/NodeObservabilityTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/NodeObservabilityTests.cs @@ -314,6 +314,53 @@ public async Task PersistArtifactAsync_delegates_to_store_with_bound_team_id() doc.GetProperty("content_type").GetString().ShouldBe("text/plain"); } + /// + /// A git node's review call whose raw /unapprove GitLab refuses while quoting the token it was presented: the + /// external_call.failed row any member reads must not carry that token. The node's run-scope redactor knows only + /// the run's own secrets, never the connection's token, so it is the provider that has to strike it — driven here + /// through the real auth resolver, GitLab provider and resilience layer, against a loopback GitLab. + /// + [Fact] + public async Task TraceExternalCallAsync_records_a_git_refusal_without_the_token_the_provider_echoed() + { + const string token = "glpat-NODE-ECHO-CANARY-9f8e7d6c5b"; + using var gitLab = new CodeSpace.IntegrationTests.Webhooks.StubProviderHost(); + gitLab.Answer("POST", "/merge_requests/7/unapprove", request => new CodeSpace.IntegrationTests.Webhooks.StubProviderHost.StubReply(401, $"{{\"message\":\"401 Unauthorized (PRIVATE-TOKEN={request.PrivateTokenHeader})\"}}")); + var (provider, context) = GitLabOver(gitLab.BaseUrl, token); + var repository = new CodeSpace.Messages.Dtos.Providers.RemoteRepository { ExternalId = "4242", NamespacePath = "acme", Name = "api", FullPath = "acme/api", DefaultBranch = "main", Visibility = CodeSpace.Messages.Enums.RepositoryVisibility.Private, WebUrl = "https://gitlab.test/acme/api" }; + var review = new CodeSpace.Messages.Dtos.Providers.SubmitPullRequestReviewInput { Verdict = CodeSpace.Messages.Enums.PullRequestReviewVerdict.RequestChanges, Body = "needs work" }; + var logger = new RecordingLogger(); + var observability = new NodeObservability(new NodeObservationBinding(logger, new RecordingArtifactStore(), RunId, NodeId, TeamId, ParentRecordId, new PersistenceSecretRedactor(["a-run-secret-not-the-token"]))); + + await Should.ThrowAsync(() => observability.TraceExternalCallAsync(target: "git.submit_review:acme/api:7", method: "submit_review", requestPayload: null, action: ct => provider.SubmitReviewAsync(context, repository, 7, review, ct), completionExtractor: _ => new ExternalCallCompletion(), cancellationToken: CancellationToken.None)); + + gitLab.Requests.ShouldContain(request => request.PrivateTokenHeader == token, "fixture check: GitLab was presented the token, so its 401 quoted it"); + var failed = logger.FailedCalls.ShouldHaveSingleItem(); + failed.Error.ShouldContain("401 Unauthorized", Case.Sensitive, "GitLab's own words still explain the failure"); + failed.Error.ShouldNotContain(token, Case.Sensitive); + } + + private static (CodeSpace.Core.Services.Providers.GitLab.GitLabRepositoryProvider Provider, CodeSpace.Core.Services.Providers.ProviderContext Context) GitLabOver(string baseUrl, string token) + { + var instance = new CodeSpace.Core.Persistence.Entities.ProviderInstance { Id = Guid.NewGuid(), TeamId = TeamId, Provider = CodeSpace.Messages.Enums.ProviderKind.GitLab, DisplayName = "loopback", BaseUrl = baseUrl }; + var credential = new CodeSpace.Core.Persistence.Entities.Credential { Id = Guid.NewGuid(), TeamId = TeamId, ProviderInstanceId = instance.Id, AuthType = CodeSpace.Messages.Enums.AuthType.Pat, DisplayName = "pat", EncryptedPayload = "unused", Status = CodeSpace.Messages.Enums.CredentialStatus.Active }; + var resolver = new CodeSpace.Core.Services.Providers.Auth.ProviderAuthResolver([new FixedGitLabToken(token)]); + var resilience = new CodeSpace.Core.Services.Providers.Resilience.ExternalCallResilience(new CodeSpace.Core.Services.Providers.Errors.ProviderErrorMapperRegistry([new CodeSpace.Core.Services.Providers.GitLab.GitLabErrorMapper()]), Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + var normalizer = new CodeSpace.Core.Services.Providers.GitLab.GitLabEventNormalizer(new CodeSpace.Core.Services.Providers.Events.ProviderEventSubscriptionRegistry([])); + var provider = new CodeSpace.Core.Services.Providers.GitLab.GitLabRepositoryProvider(resolver, resilience, new CodeSpace.Core.Services.Providers.GitLab.GitLabSignatureVerifier(), normalizer, new CodeSpace.Core.Services.Providers.GitLab.GitLabWebhookRepositoryIdentifier()); + + return (provider, new CodeSpace.Core.Services.Providers.ProviderContext(instance, credential)); + } + + private sealed class FixedGitLabToken(string token) : CodeSpace.Core.Services.Providers.Auth.IProviderAuthStrategy + { + public CodeSpace.Messages.Enums.ProviderKind Kind => CodeSpace.Messages.Enums.ProviderKind.GitLab; + + public CodeSpace.Messages.Enums.AuthType AuthType => CodeSpace.Messages.Enums.AuthType.Pat; + + public Task ResolveAsync(CodeSpace.Core.Services.Providers.ProviderContext context, CancellationToken cancellationToken) => Task.FromResult(new CodeSpace.Core.Services.Providers.Auth.ResolvedAuth { Token = token }); + } + // ─── Hand-rolled recording doubles ────────────────────────────────────────── private sealed class RecordingLogger : IRunRecordLogger diff --git a/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx b/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx index 7236dd13e..eca86ddce 100644 --- a/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx +++ b/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx @@ -9,6 +9,7 @@ import { useAddGroupAccessToken, useAddProviderInstance, useCredentialCapabiliti import { useMe } from "@/hooks/use-me"; import { TeamPermissions, useTeamPermissions } from "@/hooks/use-team-management"; import { OAuthFlowError, useOAuthFlow } from "@/hooks/use-oauth-flow"; +import { endpointChanged, providerSecretMustBeReentered, showsMaskedCredentials } from "@/lib/credentialEndpoint"; import { providerSupportsTeamServiceCredential } from "@/lib/teamCredentials"; import { Ic } from "./icons"; @@ -968,6 +969,10 @@ interface EditProviderStepProps { * ship the stored secret to the client. Leaving it blank preserves the existing value. * • Used both for "fill in missing OAuth" on a non-OAuth-ready instance AND for rotating * credentials or renaming a working one. + * • The base URL arrives with any embedded credentials masked (`***`); left as shown it is not + * sent, so the stored one is kept. Moving it to another host needs the OAuth client ID and + * secret again (the new host's app), and the server refuses the move while tokens are still + * connected through this provider — they are never sent to a host they weren't connected for. */ function EditProviderStep({ instance, onBack, onClose, onSaved }: EditProviderStepProps) { const update = useUpdateProviderInstance(); @@ -995,7 +1000,12 @@ function EditProviderStep({ instance, onBack, onClose, onSaved }: EditProviderSt const configuringFresh = !instance.oauthEnabled; const missingRequiredForConfig = configuringFresh && (!clientIdFilled || !secretFilled); const blanksOnImmutable = !displayName.trim() || !baseUrl.trim(); - const submitDisabled = update.isPending || blanksOnImmutable || oauthPairIncomplete || missingRequiredForConfig; + // A stored OAuth secret is only ever sent to the host it was entered for: moving the host needs it typed again. + const nextBaseUrl = baseUrl.trim().replace(/\/$/, ""); + const hostMoves = endpointChanged(instance.baseUrl, nextBaseUrl); + const secretMustFollow = providerSecretMustBeReentered(instance, nextBaseUrl); + const missingSecretForMove = secretMustFollow && !secretFilled; + const submitDisabled = update.isPending || blanksOnImmutable || oauthPairIncomplete || missingRequiredForConfig || missingSecretForMove; const submit = async (e: React.FormEvent) => { e.preventDefault(); @@ -1005,7 +1015,7 @@ function EditProviderStep({ instance, onBack, onClose, onSaved }: EditProviderSt // OAuth secret stays out of the payload so the backend's "leave alone" path fires. const patch: UpdateProviderInstanceRequest = {}; if (displayName.trim() !== instance.displayName) patch.displayName = displayName.trim(); - if (baseUrl.trim().replace(/\/$/, "") !== instance.baseUrl) patch.baseUrl = baseUrl.trim().replace(/\/$/, ""); + if (nextBaseUrl !== instance.baseUrl) patch.baseUrl = nextBaseUrl; if (clientIdFilled) patch.oauthClientId = clientId.trim(); if (secretFilled) patch.oauthClientSecret = clientSecret; // If we're fixing a non-OAuth provider (no existing client_id), also send default scopes. @@ -1059,6 +1069,8 @@ function EditProviderStep({ instance, onBack, onClose, onSaved }: EditProviderSt disabled={update.isPending} /> Use your install URL for self-hosted GitLab / GitHub Enterprise. + {showsMaskedCredentials(instance.baseUrl) && Credentials in the URL show as ***. Leave the URL as it is to keep them, or enter the whole URL with its credentials.} + {hostMoves && Moving to another host: tokens connected through this provider are never sent to a new host, so the save is refused while any are connected — disconnect them first, or add the new host as its own provider.{instance.oauthEnabled && " Enter the OAuth client ID and secret of the new host's app below."}}
@@ -1083,7 +1095,7 @@ function EditProviderStep({ instance, onBack, onClose, onSaved }: EditProviderSt className="cn-field-i" value={clientSecret} onChange={e => setClientSecret(e.target.value)} - placeholder={instance.oauthEnabled ? "leave blank to keep the existing value" : "paste from your OAuth app"} + placeholder={secretMustFollow ? "required for the new host" : instance.oauthEnabled ? "leave blank to keep the existing value" : "paste from your OAuth app"} spellCheck={false} autoComplete="new-password" disabled={update.isPending} diff --git a/frontend/src/api/modelCredentials.ts b/frontend/src/api/modelCredentials.ts index 465f48684..81a340b4d 100644 --- a/frontend/src/api/modelCredentials.ts +++ b/frontend/src/api/modelCredentials.ts @@ -6,7 +6,8 @@ import type { CredentialStatus } from "./types"; /** * Mirrors backend `ModelCredentialSummary` — a team's model credential as shown in settings. NEVER carries * the secret: `keyHint` is the masked tail (e.g. `····a1b2`), null for a keyless provider OR a dead key. - * `keyUnreadable` disambiguates the two. `baseUrl` is non-secret config and is shown verbatim. + * `keyUnreadable` disambiguates the two. `baseUrl` arrives with any credentials it embeds masked (`https://***:***@gw/v1?api-key=***`); + * sending that exact value back on update keeps the stored URL. */ export interface ModelCredentialSummary { id: string; @@ -29,7 +30,7 @@ export interface AddModelCredentialInput { baseUrl?: string | null; } -/** Write-only secret: omit/blank `apiKey` to keep the existing key, a value rotates it. */ +/** Write-only secret: omit/blank `apiKey` to keep the existing key, a value rotates it. Moving `baseUrl` to another scheme/host/port needs `apiKey` in the same update. */ export interface UpdateModelCredentialInput { displayName: string; apiKey?: string | null; diff --git a/frontend/src/components/connect-remote-modal.test.tsx b/frontend/src/components/connect-remote-modal.test.tsx index 2430cab98..d12be9dc2 100644 --- a/frontend/src/components/connect-remote-modal.test.tsx +++ b/frontend/src/components/connect-remote-modal.test.tsx @@ -110,3 +110,99 @@ describe("connect remote modal credential controls", () => { await waitFor(() => expect(screen.getByRole("button", { name: "Revoke" })).toBeTruthy()); }); }); + +/** + * Editing a provider's base URL. The list shows the URL with any embedded credentials masked, so the form must not send + * the masked value back as an edit; and a token or OAuth secret is only ever sent to the host it was entered for, so + * moving the host asks for the new host's OAuth app and says why the server refuses the move while tokens are connected. + */ +describe("edit provider base URL", () => { + const admin: MeTeam = { + id: "t1", slug: "acme", name: "Acme", kind: "Workspace", role: "Admin", permissions: ["repos.manage", "credentials.manage", "members.manage"], + memberCount: 3, repositoryCount: 0, projectCount: 1, workflowCount: 0, + }; + + function renderEditing(instance: ProviderInstanceSummary) { + localStorage.setItem("codespace.jwt", "test-jwt"); + localStorage.setItem("codespace.activeTeamId", admin.id); + + const patches: unknown[] = []; + const routes: Record = { + "/api/users/me": { id: "u1", email: "u@test.local", name: "Maya", teams: [admin], permissions: [], passwordMustChange: false }, + "/capabilities": { warnings: [] }, + "/api/provider-instances/defaults": { provider: instance.provider, defaultBaseUrl: "https://gitlab.com", defaultDisplayName: "GitLab", defaultOAuthScopes: ["api"], oAuthCallbackUrl: "https://codespace.test/cb" }, + "/api/provider-instances": [instance], + "/api/credentials": [], + }; + + vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + + if (init?.method === "PATCH") { + patches.push(JSON.parse(String(init.body))); + return new Response(null, { status: 204 }); + } + + const key = Object.keys(routes).find((k) => url.includes(k)); + const body = key === undefined ? undefined : routes[key]; + return new Response(body === undefined ? "" : JSON.stringify(body), { status: body === undefined ? 404 : 200, headers: { "Content-Type": "application/json" } }); + })); + + const client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } }); + + render( + + {}} /> + , + ); + + return patches; + } + + async function openEditSettings() { + await waitFor(() => expect(screen.getByTitle("More actions")).toBeTruthy()); + fireEvent.click(screen.getByTitle("More actions")); + fireEvent.click(await screen.findByRole("button", { name: /Edit settings/ })); + } + + afterEach(() => { localStorage.clear(); vi.unstubAllGlobals(); }); + + it("asks for the new host's OAuth app before a host move can be saved, and says connected tokens block it", async () => { + renderEditing({ id: "pi1", teamId: "t1", provider: "GitLab", displayName: "Acme GitLab", baseUrl: "https://gitlab.acme.test", createdDate: "2026-01-01T00:00:00Z", oauthEnabled: true }); + await openEditSettings(); + + fireEvent.change(screen.getByDisplayValue("https://gitlab.acme.test"), { target: { value: "https://attacker.test" } }); + + expect(screen.getByText(/save is refused while any are connected/)).toBeTruthy(); + const save = screen.getByRole("button", { name: /Save/ }) as HTMLButtonElement; + expect(save.disabled).toBe(true); + + fireEvent.change(screen.getAllByPlaceholderText("leave blank to keep the existing value")[0], { target: { value: "new-host-app" } }); + fireEvent.change(screen.getByPlaceholderText("required for the new host"), { target: { value: "new-host-secret" } }); + + expect(save.disabled).toBe(false); + }); + + it("keeps a path change on the same host saveable without the secret", async () => { + renderEditing({ id: "pi1", teamId: "t1", provider: "GitLab", displayName: "Acme GitLab", baseUrl: "https://gitlab.acme.test", createdDate: "2026-01-01T00:00:00Z", oauthEnabled: true }); + await openEditSettings(); + + fireEvent.change(screen.getByDisplayValue("https://gitlab.acme.test"), { target: { value: "https://gitlab.acme.test/gitlab" } }); + + expect(screen.queryByText(/save is refused while any are connected/)).toBeNull(); + expect((screen.getByRole("button", { name: /Save/ }) as HTMLButtonElement).disabled).toBe(false); + }); + + it("never sends the masked base URL back when only the name changes", async () => { + const patches = renderEditing({ id: "pi1", teamId: "t1", provider: "GitLab", displayName: "Acme GitLab", baseUrl: "https://***:***@gitlab.acme.test", createdDate: "2026-01-01T00:00:00Z", oauthEnabled: true }); + await openEditSettings(); + + expect(screen.getByText(/Leave the URL as it is to keep them/)).toBeTruthy(); + + fireEvent.change(screen.getByDisplayValue("Acme GitLab"), { target: { value: "Acme GitLab (self-hosted)" } }); + fireEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => expect(patches.length).toBe(1)); + expect(patches[0]).toEqual({ displayName: "Acme GitLab (self-hosted)" }); + }); +}); diff --git a/frontend/src/components/modelCredentials/ModelCredentialModal.test.tsx b/frontend/src/components/modelCredentials/ModelCredentialModal.test.tsx new file mode 100644 index 000000000..1d0c28434 --- /dev/null +++ b/frontend/src/components/modelCredentials/ModelCredentialModal.test.tsx @@ -0,0 +1,75 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { ModelCredentialSummary } from "@/api/modelCredentials"; + +import { ModelCredentialModal } from "./ModelCredentialModal"; + +const mocks = vi.hoisted(() => ({ updateMutate: vi.fn() })); + +vi.mock("@/hooks/use-model-credentials", async () => { + const actual = await vi.importActual("@/hooks/use-model-credentials"); + + return { + completePrice: actual.completePrice, + contextWindowFieldIssue: actual.contextWindowFieldIssue, + parseContextWindow: actual.parseContextWindow, + useAddModelCredential: () => ({ mutate: vi.fn(), isPending: false }), + useUpdateModelCredential: () => ({ mutate: mocks.updateMutate, isPending: false }), + }; +}); + +const gateway: ModelCredentialSummary = { + id: "mc1", teamId: "t1", provider: "Custom", displayName: "Gateway", + keyHint: "····a1b2", keyUnreadable: false, baseUrl: "https://***:***@gw.example/v1?api-key=***", status: "Active", createdDate: "2026-06-11T00:00:00Z", +}; + +const baseUrlInput = () => screen.getByPlaceholderText("https://your-gateway/v1"); +const saveButton = () => screen.getByRole("button", { name: "Save" }); + +describe("ModelCredentialModal — editing the endpoint", () => { + beforeEach(() => mocks.updateMutate.mockReset()); + + it("sends the masked base URL back exactly as shown when only the name changes, so the stored credentials are kept", () => { + render(); + + fireEvent.change(screen.getByDisplayValue("Gateway"), { target: { value: "Renamed" } }); + fireEvent.click(saveButton()); + + expect(mocks.updateMutate).toHaveBeenCalledWith({ id: "mc1", input: { displayName: "Renamed", apiKey: null, baseUrl: gateway.baseUrl } }, expect.anything()); + expect(screen.getByText(/Credentials in the URL show as/)).toBeInTheDocument(); + }); + + it("asks for the key again before saving a move to another host", () => { + render(); + + fireEvent.change(baseUrlInput(), { target: { value: "https://attacker.example/v1" } }); + + expect(saveButton()).toBeDisabled(); + expect(screen.getByPlaceholderText("Required for the new endpoint")).toBeInTheDocument(); + expect(screen.getByText(/The endpoint changed — enter the key for it/)).toBeInTheDocument(); + + fireEvent.change(screen.getByPlaceholderText("Required for the new endpoint"), { target: { value: "sk-new-gateway-key" } }); + fireEvent.click(saveButton()); + + expect(mocks.updateMutate).toHaveBeenCalledWith({ id: "mc1", input: { displayName: "Gateway", apiKey: "sk-new-gateway-key", baseUrl: "https://attacker.example/v1" } }, expect.anything()); + }); + + it("keeps the key optional for a path-only change on the same endpoint", () => { + render(); + + fireEvent.change(baseUrlInput(), { target: { value: "https://gw.example/v2" } }); + + expect(saveButton()).toBeEnabled(); + expect(screen.queryByText(/The endpoint changed/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Credentials in the URL show as/)).not.toBeInTheDocument(); + }); + + it("never asks a keyless credential for a key", () => { + render(); + + fireEvent.change(screen.getByPlaceholderText("http://localhost:11434"), { target: { value: "http://ollama.internal:11434" } }); + + expect(saveButton()).toBeEnabled(); + }); +}); diff --git a/frontend/src/components/modelCredentials/ModelCredentialModal.tsx b/frontend/src/components/modelCredentials/ModelCredentialModal.tsx index 4ab7a68f8..82314bfaa 100644 --- a/frontend/src/components/modelCredentials/ModelCredentialModal.tsx +++ b/frontend/src/components/modelCredentials/ModelCredentialModal.tsx @@ -5,6 +5,7 @@ import { Ic } from "@/_imported/ai-code-space/icons"; import type { ModelCredentialSummary } from "@/api/modelCredentials"; import { ApiError } from "@/api/request"; import { completePrice, contextWindowFieldIssue, parseContextWindow, useAddModelCredential, useUpdateModelCredential } from "@/hooks/use-model-credentials"; +import { keyMustBeReentered, showsMaskedCredentials } from "@/lib/credentialEndpoint"; import { PROVIDER_FORMS, providerForm } from "@/lib/providerForms"; import { ModelRowsEditor, type ModelRow } from "./ModelRowsEditor"; @@ -18,8 +19,9 @@ interface ModelCredentialModalProps { /** * Add / edit a model credential. Mirrors the warm-theme `.mdl` + `wf-form` shell (see IdentityLinkModal), * and renders its fields from the per-provider form variant (lib/providerForms): a masked secret (write-only - * on edit — blank keeps the current key), a base URL (required for gateway/keyless providers, optional - * otherwise), and a provider picker when adding. + * on edit — blank keeps the current key, unless the edit moves the endpoint, which needs the key again), a base + * URL (required for gateway/keyless providers, optional otherwise; shown with any embedded credentials masked as + * `***`, and sent back as shown to keep them), and a provider picker when adding. */ export function ModelCredentialModal({ editing, onClose }: ModelCredentialModalProps) { const isEdit = editing != null; @@ -46,8 +48,12 @@ export function ModelCredentialModal({ editing, onClose }: ModelCredentialModalP const keyField = form?.fields.find(f => f.key === "apiKey"); const baseUrlField = form?.fields.find(f => f.key === "baseUrl"); const baseUrlRequired = baseUrlField?.required ?? false; - // Key is mandatory only when ADDING a key-based provider; on edit a blank key keeps the existing one. - const keyRequired = !isEdit && !keyless && (keyField?.required ?? false); + // Key is mandatory when ADDING a key-based provider. On edit a blank key keeps the existing one — unless the edit + // moves the endpoint (scheme/host/port): a stored key is only ever sent where it was entered, so the move carries + // the key with it or the server refuses it (model_endpoint_key_required). + const keyMustFollow = isEdit && !keyless && keyMustBeReentered(editing, baseUrl); + const keyRequired = keyMustFollow || (!isEdit && !keyless && (keyField?.required ?? false)); + const baseUrlShowsMasked = isEdit && showsMaskedCredentials(editing.baseUrl); const requiredOk = displayName.trim() !== "" && @@ -117,10 +123,10 @@ export function ModelCredentialModal({ editing, onClose }: ModelCredentialModalP value={apiKey} onChange={(e) => setApiKey(e.target.value)} onKeyDown={(e) => { if (e.key === "Enter") submit(); }} - placeholder={isEdit ? `${editing.keyHint ?? "····"} — leave blank to keep` : keyField.placeholder} + placeholder={keyMustFollow ? "Required for the new endpoint" : isEdit ? `${editing.keyHint ?? "····"} — leave blank to keep` : keyField.placeholder} autoFocus={isEdit} /> - {isEdit ? "Leave blank to keep the current key, or paste a new one to rotate." : "Stored encrypted; we never show it again."} + {keyMustFollow ? "The endpoint changed — enter the key for it. A stored key is only ever sent to the endpoint it was entered for." : isEdit ? "Leave blank to keep the current key, or paste a new one to rotate." : "Stored encrypted; we never show it again."} )} @@ -128,6 +134,7 @@ export function ModelCredentialModal({ editing, onClose }: ModelCredentialModalP )} diff --git a/frontend/src/lib/credentialEndpoint.test.ts b/frontend/src/lib/credentialEndpoint.test.ts new file mode 100644 index 000000000..d38cb0161 --- /dev/null +++ b/frontend/src/lib/credentialEndpoint.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; + +import { endpointChanged, keyMustBeReentered, providerSecretMustBeReentered, showsMaskedCredentials } from "./credentialEndpoint"; + +describe("endpointChanged", () => { + it.each([ + ["https://gw.example/v1", "https://gw.example/v2"], + ["https://gw.example/v1", "https://GW.example:443/v1"], + ["https://***:***@gw.example/v1?api-key=***", "https://other:creds@gw.example/v1"], + [null, ""], + ["", null], + ["gw.example:4000", "gw.example:4000"], + ])("keeps %s → %s on the same endpoint (scheme, host, port)", (original, next) => { + expect(endpointChanged(original, next)).toBe(false); + }); + + it.each([ + ["https://gw.example/v1", "https://attacker.example/v1"], + ["https://gw.example/v1", "http://gw.example/v1"], + ["https://gw.example/v1", "https://gw.example:8443/v1"], + [null, "https://gw.example/v1"], + ["https://gw.example/v1", ""], + ["gw.example:4000", "gw.example:5000"], + ])("moves %s → %s to another endpoint", (original, next) => { + expect(endpointChanged(original, next)).toBe(true); + }); +}); + +describe("keyMustBeReentered", () => { + it("asks for the key when a stored key would follow the endpoint somewhere new", () => { + expect(keyMustBeReentered({ keyHint: "····a1b2", keyUnreadable: false, baseUrl: "https://gw.example/v1" }, "https://attacker.example/v1")).toBe(true); + }); + + it("asks for it for an unreadable stored key too — it is still a stored key", () => { + expect(keyMustBeReentered({ keyHint: null, keyUnreadable: true, baseUrl: "https://gw.example/v1" }, "https://other.example/v1")).toBe(true); + }); + + it("never asks for a keyless credential", () => { + expect(keyMustBeReentered({ keyHint: null, keyUnreadable: false, baseUrl: "http://localhost:11434" }, "http://ollama.internal:11434")).toBe(false); + }); + + it("never asks for a path-only change", () => { + expect(keyMustBeReentered({ keyHint: "····a1b2", keyUnreadable: false, baseUrl: "https://gw.example/v1" }, "https://gw.example/v2")).toBe(false); + }); +}); + +describe("providerSecretMustBeReentered", () => { + it("asks for the OAuth secret when a stored one would follow the provider to another host", () => { + expect(providerSecretMustBeReentered({ oauthEnabled: true, baseUrl: "https://gitlab.acme.test" }, "https://attacker.test")).toBe(true); + }); + + it("never asks when no secret is stored", () => { + expect(providerSecretMustBeReentered({ oauthEnabled: false, baseUrl: "https://gitlab.acme.test" }, "https://gitlab.new.test")).toBe(false); + }); + + it("never asks for a path change or the masked URL sent back", () => { + expect(providerSecretMustBeReentered({ oauthEnabled: true, baseUrl: "https://gitlab.acme.test" }, "https://gitlab.acme.test/gitlab")).toBe(false); + expect(providerSecretMustBeReentered({ oauthEnabled: true, baseUrl: "https://***:***@gitlab.acme.test" }, "https://***:***@gitlab.acme.test")).toBe(false); + }); +}); + +describe("showsMaskedCredentials", () => { + it.each([ + ["https://***:***@gitlab.acme.test", true], + ["https://gitlab.acme.test/api?private_token=***", true], + ["https://gitlab.acme.test", false], + [null, false], + ])("reads %s as carrying masked credentials: %s", (url, expected) => { + expect(showsMaskedCredentials(url)).toBe(expected); + }); +}); diff --git a/frontend/src/lib/credentialEndpoint.ts b/frontend/src/lib/credentialEndpoint.ts new file mode 100644 index 000000000..21441cc32 --- /dev/null +++ b/frontend/src/lib/credentialEndpoint.ts @@ -0,0 +1,50 @@ +import type { ModelCredentialSummary } from "@/api/modelCredentials"; +import type { ProviderInstanceSummary } from "@/api/types"; + +/** + * The endpoint a stored secret is bound to — scheme, host and port, i.e. where the secret is sent. Mirrors the backend + * rule (`CredentialUrl.SameEndpoint`): a model credential's update that moves it must carry the key again + * (`model_endpoint_key_required`), and a git provider's must carry its OAuth client secret again + * (`provider_endpoint_secret_required`) and is refused outright while tokens are connected through it + * (`provider_endpoint_credentials_bound`). Path, query and userinfo are not part of it, so a path-only edit (or the + * masked `***` credentials the lists show) never counts as a move. + */ +function endpointOf(baseUrl: string | null | undefined): string | null { + const trimmed = baseUrl?.trim() ?? ""; + if (trimmed === "") return null; + + try { + const url = new URL(trimmed); + if (url.protocol !== "http:" && url.protocol !== "https:") return trimmed; + + return `${url.protocol}//${url.hostname.toLowerCase()}:${url.port || (url.protocol === "https:" ? "443" : "80")}`; + } catch { + return trimmed; // not an absolute http(s) URL — it only matches itself + } +} + +/** Whether `next` sends a key somewhere other than `original` does. Blank on both sides is the provider default on both. */ +export function endpointChanged(original: string | null | undefined, next: string | null | undefined): boolean { + return endpointOf(original) !== endpointOf(next); +} + +/** Whether saving `nextBaseUrl` on this credential needs the key typed again: a key is stored (readable or not) and the endpoint moves. */ +export function keyMustBeReentered(credential: Pick, nextBaseUrl: string | null | undefined): boolean { + const hasStoredKey = credential.keyHint != null || credential.keyUnreadable; + + return hasStoredKey && endpointChanged(credential.baseUrl, nextBaseUrl); +} + +/** Whether a URL the list showed still carries masked (`***`) embedded credentials — sent back unchanged it keeps them; edited, they must be typed in full. */ +export function showsMaskedCredentials(url: string | null | undefined): boolean { + return url?.includes("***") ?? false; +} + +/** + * Whether saving `nextBaseUrl` on a git provider connection moves it to another host, which its stored OAuth client + * secret may not follow: the save needs the secret (the new host's OAuth app's, with its client ID) typed again. The + * edit form changes only the base URL, so this is the backend's `ProviderInstanceService.EndpointMoves` for that one URL. + */ +export function providerSecretMustBeReentered(instance: Pick, nextBaseUrl: string): boolean { + return instance.oauthEnabled && endpointChanged(instance.baseUrl, nextBaseUrl); +}