diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index db205b06..a1497801 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -399,7 +399,9 @@ it links: `SkipCertificateValidation` (Docker instances) and `AllowInsecureHttp` (Portainer) exist for trusted/self-signed/local setups; how each is validated and reported is in [Docker instances](areas/docker-instances.md) - and [Container actions](areas/container-actions.md). + and [Container actions](areas/container-actions.md). URLs with user info + are logged, stored and shown with it masked as `***` + ([Telemetry](areas/telemetry.md), [record 0005](decisions/0005-urls-are-output-with-user-info-masked.md)). - **Pagination stays on its origin.** `DockerHubClient`, `OciRegistryClient` and `DockerScoutVulnerabilityProvider` follow a next-page link only on the origin of the listing (`Infrastructure\PaginationLinkGuard`), so a remote diff --git a/docs/areas/container-actions.md b/docs/areas/container-actions.md index 2b2118b5..a9a17284 100644 --- a/docs/areas/container-actions.md +++ b/docs/areas/container-actions.md @@ -17,8 +17,9 @@ transport security; the UI is the intended caller of the actions. - A Docker instance has at most one Portainer endpoint. It is synchronized together with the instance ([Docker instances](docker-instances.md)): - when Portainer is enabled, the stored endpoint is created or updated with the name ` Portainer`, - the trimmed `BaseUrl` and the configured `EndpointId`, and marked enabled; - - when Portainer is disabled, an existing stored endpoint is kept but marked disabled; + the trimmed `BaseUrl` (its user info masked as `***`) and the configured `EndpointId`, and marked enabled; + - when Portainer is disabled, an existing stored endpoint is kept but marked disabled, and its stored base URL is rewritten with the user info + masked; - when the Docker instance is removed from configuration, its Portainer endpoint and action runs are deleted with it. - Credentials (`Username`, `Password`, `ApiToken`) are never stored in the database; they are read from configuration at the time of each call. @@ -39,7 +40,7 @@ transport security; the UI is the intended caller of the actions. - Otherwise `Username` and `Password` are posted to Portainer's authentication endpoint and the returned JWT is used as a bearer token. The login is repeated for every operation; tokens are not cached. - A login that answers non-success, or answers success without a token, ends the operation with an authentication - error that names the Portainer base URL and the HTTP status code; credentials never appear in it or in the log. + error that names the Portainer base URL (user info masked as `***`) and the HTTP status code; credentials never appear in it or in the log. ### Capability check @@ -85,7 +86,7 @@ transport security; the UI is the intended caller of the actions. absolute `http`/`https` URI; `BaseUrl` is `http` without `AllowInsecureHttp: true`; neither `ApiToken` nor both `Username` and `Password` are set; `RequestTimeoutSeconds` is outside 1–300. - A disabled Portainer section is not validated at all; its values may be incomplete. -- Error messages and log entries name the Docker instance, the Portainer base URL, the action, the container name and +- Error messages and log entries name the Docker instance, the Portainer base URL (user info masked as `***`), the action, the container name and HTTP status codes. They never contain the password, the API token or the JWT. ### Known gaps diff --git a/docs/areas/docker-instances.md b/docs/areas/docker-instances.md index f1d93145..2a31c759 100644 --- a/docs/areas/docker-instances.md +++ b/docs/areas/docker-instances.md @@ -23,7 +23,7 @@ on the snapshots and samples it produces. `DockerUpdateGuard:Scanning:DiscoveryIntervalMinutes` tick, and at the start of every runtime container scan and every resource-statistics collection (schedules: [Scan engine](scan-engine.md)). A synchronization: - creates a record for each new name, with registration source *configuration file*; - - updates endpoint URI (trimmed `BaseUrl`), connection kind, enabled flag and `SkipCertificateValidation` of each + - updates endpoint URI (trimmed `BaseUrl`, its user info masked as `***`), connection kind, enabled flag and `SkipCertificateValidation` of each existing one; - **deletes** every stored instance whose name is no longer configured, immediately and without waiting for retention, together with its scan runs, container snapshots, resource samples, Portainer endpoint and action @@ -147,6 +147,8 @@ on the snapshots and samples it produces. - A non-success Engine answer is reported as *failed* with the instance name, the HTTP status code and the response body. Unexpected errors are reported with the instance name and the error message. These messages can end up in a scan run's error message and are shown in the UI. +- A message or log entry that names an endpoint shows its URL with the user info replaced by `***`; connecting still + uses the configured URL. - A resource collection that fails for an instance (list request failed or timed out, or any stats request timed out or broke) records **no** samples for that instance in that round and logs a warning; other instances are not affected. diff --git a/docs/areas/registries.md b/docs/areas/registries.md index 5cd87278..ea289a25 100644 --- a/docs/areas/registries.md +++ b/docs/areas/registries.md @@ -163,7 +163,7 @@ retry policy and the failure model. of that value, the total again capped at 30 seconds. - Every method is retried, including requests with a body (the body is buffered so it can be resent). - When the retries are used up the last response is returned to the caller, or the last error is raised. -- Each retry is logged as a warning with the method, the URL without its query, the status or error, the attempt +- Each retry is logged as a warning with the method, the URL without its user info and query, the status or error, the attempt number and the wait. - `DockerUpdateGuard:DockerHub:RequestTimeoutSeconds` (default 30, range 1–300) is the total time one request may take including all retries and waits, for Docker Hub, other registries and the release feeds. A request that runs diff --git a/docs/areas/telemetry.md b/docs/areas/telemetry.md index 2a3fc56c..a448705b 100644 --- a/docs/areas/telemetry.md +++ b/docs/areas/telemetry.md @@ -102,8 +102,12 @@ Every exported signal carries the same resource: nor the database connection string. Credentials travel only in request headers, and headers are not recorded. - Database statements are logged without parameter values. - What logs and spans may carry about external systems: configured instance names, image references, configured base - URLs, the Docker Hub account name, HTTP methods and status codes, the scheme, host and port of a rejected - next-page link, counts, durations and error messages from external tools. Retry log lines carry the request URL without its query string. + URLs with any user info replaced by `***`, the Docker Hub account name, HTTP methods and status codes, the scheme, + host and port of a rejected next-page link, counts, durations and error messages from external tools. Retry log + lines carry the request URL without its user info and query string. +- HTTP client spans (`url.full`) and the HTTP client log lines carry scheme, host, port and path only, with query + values as `*`, unless the platform's URI redaction is switched off + (`DOTNET_SYSTEM_NET_HTTP_DISABLEURIREDACTION`). Decision: [0005](../decisions/0005-urls-are-output-with-user-info-masked.md). ## Limits and errors @@ -123,13 +127,10 @@ Every exported signal carries the same resource: - `dockerupdateguard.runtime_containers` counts every container that still has a stored snapshot, including containers that have since stopped or been removed, until cleanup deletes their snapshots; it does not count only the currently known containers. Tracked in [#294](https://github.com/LarsLaskowski/DockerUpdateGuard/issues/294). -- Configured base URLs (Docker instance, Portainer) are logged verbatim, and retry log lines keep the user-info part - of a URL, so credentials embedded in a URL (`https://user:secret@host`) reach the logs. - Tracked in [#295](https://github.com/LarsLaskowski/DockerUpdateGuard/issues/295). ## Related decisions -None yet. +- [0005](../decisions/0005-urls-are-output-with-user-info-masked.md) — URLs are output with their user info masked. ## Not here diff --git a/docs/areas/web-ui.md b/docs/areas/web-ui.md index c95b6749..95abab93 100644 --- a/docs/areas/web-ui.md +++ b/docs/areas/web-ui.md @@ -115,7 +115,8 @@ never their values. - The UI never shows configured secrets: the Docker Hub token, Portainer credentials and token and the database connection string are neither displayed nor sent to the browser. Pages show only whether an integration is configured, and the Docker Hub user name as the title of the My Images list. -- Docker instance endpoints are shown as configured (see *Known gaps*). +- Docker instance endpoints are shown with the user info of their URL replaced by `***` + (`https://***@docker.example:2376/`), also for rows stored before the masking existed. ## Limits and errors @@ -139,15 +140,13 @@ never their values. - A detail route with a well-formed identifier that does not exist (for example a deleted observed image) keeps showing the loading skeleton instead of a not-found state. Tracked in [#40](https://github.com/LarsLaskowski/DockerUpdateGuard/issues/40). -- Docker instance endpoints are not masked: an endpoint that embeds credentials as user information in its URL is - shown with them. Tracked in [#295](https://github.com/LarsLaskowski/DockerUpdateGuard/issues/295). - The Scan History and dashboard descriptions mention discovery and cleanup runs, although those jobs record no scan runs ([Scan engine](scan-engine.md)). Tracked in [#278](https://github.com/LarsLaskowski/DockerUpdateGuard/issues/278). ## Related decisions -None yet. +- [0005](../decisions/0005-urls-are-output-with-user-info-masked.md) — URLs are output with their user info masked. ## Not here diff --git a/docs/decisions/0005-urls-are-output-with-user-info-masked.md b/docs/decisions/0005-urls-are-output-with-user-info-masked.md new file mode 100644 index 00000000..b991081a --- /dev/null +++ b/docs/decisions/0005-urls-are-output-with-user-info-masked.md @@ -0,0 +1,55 @@ +# 0005: URLs are output with their user info masked + +- **Status:** Accepted +- **Date:** 2026-10-24 +- **Area:** Telemetry +- **Source:** Issue #295 +- **Supersedes:** — + +## Context + +Configuration validation accepts Docker instance and Portainer base URLs that carry user info +(`https://user:secret@host`), and the application wrote those URLs as they stand into log messages and their +attributes, into exception and result messages that end up in scan-run errors and on the pages, into the database +and onto the Docker Instances pages; the retry log dropped only the query string. Secrets in configuration must +never be logged, shown in the UI or put into exception messages or telemetry (`.squad/project.md`, *Security +areas*). The clients still have to connect with the URL as configured. + +## Options considered + +1. **Mask the user info with `***` wherever a URL is output, keep the configured value for connecting** — operators + still see that credentials are configured and which host is meant; one helper serves every output site, and the + stored display values are masked on write and again on display. +2. **Drop the user info entirely** — equally safe, but the output no longer shows that the configured URL carries + credentials, which hides a configuration the operator may want to move into a proper credential setting. +3. **Reject URLs with user info in configuration validation** — removes the problem at its source, but makes + startup fail for configurations that work today; a product decision beyond this bug. +4. **Redact in a log processor or enricher** — covers exported logs only: the console logger, exception and result + messages, scan-run errors, the database and the UI would still carry the secret. +5. **Mask only the leading `scheme://userinfo@` with a regular expression, as the issue sketched** — a password with + an unescaped `@` (`https://user:p@ss@host`, which the URI parser rejects) would leave the part after the first + `@` visible, and one with a `/` would not be matched at all. +6. **Mask only on display, store the configured URL** — keeps the secret at rest in the database. + +## Decision + +Option 1. A value with exactly one `@` that the platform's URI parser accepts as `scheme://` with non-empty user info +is output rebuilt from its parts with the user info replaced by `***`; every other value that contains `@` is masked +fail-closed from after its `scheme://` (or from its start) up to its last `@`, because the parser ends the authority at +the first `/`, `?` or `#` and so reads a password such as `admin:#secret@host` as a host without user info; text +without `@` is output unchanged. The rules and where they apply are specified in +[Telemetry](../areas/telemetry.md#credentials-and-external-data) and [Web UI](../areas/web-ui.md#secrets). + +## Consequences + +- No configured URL credential reaches logs, telemetry attributes, messages, scan runs, the database or the UI + through the paths this change covers; the HTTP client spans and the HTTP client factory's own log lines already + omit user info, because the platform builds their URL from scheme, host, port and path. +- A URL with user info is shown in the parser's normalized form (lower-case host, default port dropped, trailing + slash added); a URL without user info is shown exactly as configured. +- A value with an `@` that is not user info (`https://host.example/@scope/pkg` → `https://***@scope/pkg`, or a second + `@` in the path) is over-masked; accepted, it fails closed. Masking an already masked value changes nothing. +- Messages an external tool writes (Trivy's error output) are passed on as they stand; masking free text is not part + of this decision. +- Rows stored before the change are masked on display and rewritten masked by the next synchronization; scan-run + error messages stored before the change are not rewritten. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 112a143e..e5ca06b6 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -57,4 +57,5 @@ once in the [area document](../areas/README.md) of the record's area, and the re | [0002](0002-squash-merge-pull-requests.md) | Squash-merge pull requests | Accepted | 2026-10-03 | | [0003](0003-squad-working-records-off-main.md) | Squad working records stay off main, and product PRs never change the squad | Accepted | 2026-10-03 | | [0004](0004-pagination-follows-only-same-origin-links.md) | Pagination follows only same-origin links | Accepted | 2026-10-24 | +| [0005](0005-urls-are-output-with-user-info-masked.md) | URLs are output with their user info masked | Accepted | 2026-10-24 | diff --git a/src/DockerUpdateGuard/Docker/DockerInstanceClient.cs b/src/DockerUpdateGuard/Docker/DockerInstanceClient.cs index ab5a6158..539f8f69 100644 --- a/src/DockerUpdateGuard/Docker/DockerInstanceClient.cs +++ b/src/DockerUpdateGuard/Docker/DockerInstanceClient.cs @@ -471,7 +471,7 @@ private static string GetNamedPipeName(Uri pipeUri) if (string.IsNullOrWhiteSpace(pipePath)) { - throw new InvalidOperationException($"The Docker named pipe URI '{pipeUri}' does not contain a pipe name"); + throw new InvalidOperationException($"The Docker named pipe URI '{UrlRedaction.RedactUserInfo(pipeUri)}' does not contain a pipe name"); } return pipePath; @@ -980,9 +980,9 @@ public async Task>.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{instanceOptions.BaseUrl}'"); + return ExternalOperationResult>.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{UrlRedaction.RedactUserInfo(instanceOptions.BaseUrl)}'"); } using var activity = DockerUpdateGuardTelemetry.ActivitySource.StartActivity(TelemetryActivityNames.DockerEngineRequest, ActivityKind.Client); @@ -1060,7 +1060,7 @@ public async Task>.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{instanceOptions.BaseUrl}'"); + return ExternalOperationResult>.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{UrlRedaction.RedactUserInfo(instanceOptions.BaseUrl)}'"); } try @@ -1149,7 +1149,7 @@ public async Task> InspectImageA if (TryCreateEngineUri(instanceOptions, out var engineUri) == false || engineUri is null) { - return ExternalOperationResult.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{instanceOptions.BaseUrl}'"); + return ExternalOperationResult.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{UrlRedaction.RedactUserInfo(instanceOptions.BaseUrl)}'"); } try @@ -1186,7 +1186,7 @@ public async Task>.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{instanceOptions.BaseUrl}'"); + return ExternalOperationResult>.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{UrlRedaction.RedactUserInfo(instanceOptions.BaseUrl)}'"); } try @@ -1220,7 +1220,7 @@ public async Task> GetHostMemoryTotalAsync(DockerI if (TryCreateEngineUri(instanceOptions, out var engineUri) == false || engineUri is null) { - return ExternalOperationResult.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{instanceOptions.BaseUrl}'"); + return ExternalOperationResult.Unsupported($"Docker instance '{instanceOptions.Name}' uses an unsupported endpoint '{UrlRedaction.RedactUserInfo(instanceOptions.BaseUrl)}'"); } try diff --git a/src/DockerUpdateGuard/Images/InstanceDiscoveryService.cs b/src/DockerUpdateGuard/Images/InstanceDiscoveryService.cs index 921c01dd..097a4d55 100644 --- a/src/DockerUpdateGuard/Images/InstanceDiscoveryService.cs +++ b/src/DockerUpdateGuard/Images/InstanceDiscoveryService.cs @@ -2,6 +2,7 @@ using DockerUpdateGuard.Data; using DockerUpdateGuard.Data.Entities; using DockerUpdateGuard.Images.Interfaces; +using DockerUpdateGuard.Infrastructure; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; @@ -106,7 +107,7 @@ private void SynchronizeConfiguredInstance(DockerInstanceOptions configuredInsta existingInstances.Add(existingInstance); } - existingInstance.EndpointUri = configuredInstance.BaseUrl.Trim(); + existingInstance.EndpointUri = UrlRedaction.RedactUserInfo(configuredInstance.BaseUrl.Trim()); existingInstance.ConnectionKind = MapConnectionKind(configuredInstance); existingInstance.IsEnabled = configuredInstance.Enabled; existingInstance.SkipCertificateValidation = configuredInstance.SkipCertificateValidation; @@ -115,7 +116,7 @@ private void SynchronizeConfiguredInstance(DockerInstanceOptions configuredInsta if (configuredInstance.Enabled && existingInstance.ConnectionKind == DockerConnectionKind.NotSet) { - _logger.DockerInstanceConfigurationUnsupported(configuredInstance.Name, configuredInstance.BaseUrl); + _logger.DockerInstanceConfigurationUnsupported(configuredInstance.Name, UrlRedaction.RedactUserInfo(configuredInstance.BaseUrl)); } SynchronizePortainerEndpoint(existingInstance, configuredInstance); @@ -132,6 +133,7 @@ private void SynchronizePortainerEndpoint(DockerInstance existingInstance, Docke { if (existingInstance.PortainerEndpoint is not null) { + existingInstance.PortainerEndpoint.BaseUrl = UrlRedaction.RedactUserInfo(existingInstance.PortainerEndpoint.BaseUrl); existingInstance.PortainerEndpoint.IsEnabled = false; existingInstance.PortainerEndpoint.UpdatedAtUtc = DateTimeOffset.UtcNow; } @@ -152,7 +154,7 @@ private void SynchronizePortainerEndpoint(DockerInstance existingInstance, Docke } existingInstance.PortainerEndpoint.Name = $"{existingInstance.Name} Portainer"; - existingInstance.PortainerEndpoint.BaseUrl = configuredInstance.Portainer.BaseUrl?.Trim() ?? string.Empty; + existingInstance.PortainerEndpoint.BaseUrl = UrlRedaction.RedactUserInfo(configuredInstance.Portainer.BaseUrl?.Trim()); existingInstance.PortainerEndpoint.ExternalEndpointId = configuredInstance.Portainer.EndpointId; existingInstance.PortainerEndpoint.IsEnabled = true; existingInstance.PortainerEndpoint.UpdatedAtUtc = DateTimeOffset.UtcNow; diff --git a/src/DockerUpdateGuard/Infrastructure/TransientHttpRetryHandler.cs b/src/DockerUpdateGuard/Infrastructure/TransientHttpRetryHandler.cs index f96f074b..2994e9fc 100644 --- a/src/DockerUpdateGuard/Infrastructure/TransientHttpRetryHandler.cs +++ b/src/DockerUpdateGuard/Infrastructure/TransientHttpRetryHandler.cs @@ -128,10 +128,13 @@ private static string GetRequestTarget(HttpRequestMessage request) if (requestUri.IsAbsoluteUri) { - return requestUri.GetLeftPart(UriPartial.Path); + return UrlRedaction.RedactUserInfo(requestUri.GetLeftPart(UriPartial.Path)); } - return requestUri.OriginalString; + var originalString = requestUri.OriginalString; + var queryOrFragment = originalString.IndexOfAny(['?', '#']); + + return UrlRedaction.RedactUserInfo(queryOrFragment < 0 ? originalString : originalString[..queryOrFragment]); } #endregion // Static methods diff --git a/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs b/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs new file mode 100644 index 00000000..320ac4ba --- /dev/null +++ b/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs @@ -0,0 +1,108 @@ +namespace DockerUpdateGuard.Infrastructure; + +/// +/// Masks the user info of URLs before they reach logs, messages, stored values or the UI +/// +internal static class UrlRedaction +{ + #region Constants + + /// + /// Text that replaces the user info of a URL + /// + public const string Mask = "***"; + + #endregion // Constants + + #region Methods + + /// + /// Replace the user info of a URL text with the mask + /// + /// URL text + /// URL text with masked user info, empty for null or empty input + public static string RedactUserInfo(string? url) + { + if (string.IsNullOrEmpty(url)) + { + return string.Empty; + } + + var last = url.LastIndexOf('@'); + + if (last < 0) + { + return url; + } + + if (url.IndexOf('@') == last + && Uri.TryCreate(url, UriKind.Absolute, out var uri) + && url.Trim().StartsWith($"{uri.Scheme}://", StringComparison.OrdinalIgnoreCase) + && string.IsNullOrEmpty(uri.UserInfo) == false) + { + var remainder = uri.GetComponents(UriComponents.Host | UriComponents.Port | UriComponents.PathAndQuery | UriComponents.Fragment, UriFormat.UriEscaped); + + return $"{uri.Scheme}://{Mask}@{remainder}"; + } + + var start = 0; + var separator = url.IndexOf("://", StringComparison.Ordinal); + + if (separator > 0 + && separator < last + && IsValidScheme(url.AsSpan(0, separator))) + { + start = separator + 3; + } + + return string.Concat(url.AsSpan(0, start), Mask, url.AsSpan(last)); + } + + /// + /// Replace the user info of a URI with the mask + /// + /// URI + /// URI text with masked user info, empty for null + public static string RedactUserInfo(Uri? uri) + { + if (uri is null) + { + return string.Empty; + } + + return RedactUserInfo(uri.IsAbsoluteUri ? uri.AbsoluteUri : uri.OriginalString); + } + + #endregion // Methods + + #region Static methods + + /// + /// Check whether a text is a valid URI scheme (RFC 3986: ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )) + /// + /// Text before the scheme separator + /// True when the text is a valid scheme + private static bool IsValidScheme(ReadOnlySpan scheme) + { + if (scheme.IsEmpty + || char.IsAsciiLetter(scheme[0]) == false) + { + return false; + } + + foreach (var character in scheme) + { + if (char.IsAsciiLetterOrDigit(character) == false + && character != '+' + && character != '-' + && character != '.') + { + return false; + } + } + + return true; + } + + #endregion // Static methods +} \ No newline at end of file diff --git a/src/DockerUpdateGuard/Portainer/PortainerClient.cs b/src/DockerUpdateGuard/Portainer/PortainerClient.cs index 7f5d627c..02b8493d 100644 --- a/src/DockerUpdateGuard/Portainer/PortainerClient.cs +++ b/src/DockerUpdateGuard/Portainer/PortainerClient.cs @@ -2,6 +2,7 @@ using System.Text.Json; using DockerUpdateGuard.Configuration; +using DockerUpdateGuard.Infrastructure; using DockerUpdateGuard.Portainer.Data; using DockerUpdateGuard.Portainer.Interfaces; @@ -150,6 +151,7 @@ private async Task CreateAuthenticatedClientAsync(PortainerOptions o if (string.IsNullOrWhiteSpace(options.Username) == false && string.IsNullOrWhiteSpace(options.Password) == false) { + var maskedBaseUrl = UrlRedaction.RedactUserInfo(options.BaseUrl); var loginBody = new PortainerLoginRequest(options.Username, options.Password); using var loginResponse = await client.PostAsJsonAsync("/api/auth", loginBody, cancellationToken).ConfigureAwait(false); @@ -163,16 +165,16 @@ private async Task CreateAuthenticatedClientAsync(PortainerOptions o } else { - _logger.PortainerAuthFailed(options.BaseUrl ?? string.Empty); + _logger.PortainerAuthFailed(maskedBaseUrl); - throw new InvalidOperationException($"Portainer authentication succeeded but returned no JWT token for '{options.BaseUrl}'"); + throw new InvalidOperationException($"Portainer authentication succeeded but returned no JWT token for '{maskedBaseUrl}'"); } } else { - _logger.PortainerAuthFailed(options.BaseUrl ?? string.Empty); + _logger.PortainerAuthFailed(maskedBaseUrl); - throw new InvalidOperationException($"Portainer authentication failed with HTTP {(int)loginResponse.StatusCode} for '{options.BaseUrl}'"); + throw new InvalidOperationException($"Portainer authentication failed with HTTP {(int)loginResponse.StatusCode} for '{maskedBaseUrl}'"); } } diff --git a/src/DockerUpdateGuard/UI/ApplicationViewService.cs b/src/DockerUpdateGuard/UI/ApplicationViewService.cs index b34dd4a4..1af6e6dc 100644 --- a/src/DockerUpdateGuard/UI/ApplicationViewService.cs +++ b/src/DockerUpdateGuard/UI/ApplicationViewService.cs @@ -5,6 +5,7 @@ using DockerUpdateGuard.Images.Data; using DockerUpdateGuard.Images.Helper; using DockerUpdateGuard.Images.Interfaces; +using DockerUpdateGuard.Infrastructure; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; @@ -2098,7 +2099,7 @@ public async Task> GetDockerInstancesA { Id = entity.Id, Name = entity.Name, - EndpointUri = entity.EndpointUri, + EndpointUri = UrlRedaction.RedactUserInfo(entity.EndpointUri), ConnectionKind = entity.ConnectionKind.ToString(), PortainerEnabled = entity.PortainerEndpoint is not null && entity.PortainerEndpoint.IsEnabled, LatestScanStatus = GetLatestRuntimeScanStatus(entity.Id), @@ -2145,7 +2146,7 @@ public async Task> GetDockerInstancesA { Id = dockerInstance.Id, Name = dockerInstance.Name, - EndpointUri = dockerInstance.EndpointUri, + EndpointUri = UrlRedaction.RedactUserInfo(dockerInstance.EndpointUri), ConnectionKind = dockerInstance.ConnectionKind.ToString(), PortainerEnabled = dockerInstance.PortainerEndpoint is not null && dockerInstance.PortainerEndpoint.IsEnabled, LatestScanStatus = GetLatestRuntimeScanStatus(dockerInstanceId), diff --git a/src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs b/src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs index 49a6767d..20cf693d 100644 --- a/src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs @@ -2998,5 +2998,132 @@ public async Task ApplicationViewServiceVulnerabilityScheduleWithDisabledScannin Assert.IsNull(schedule.NextVulnerabilityRefreshUtc, "The next scheduled refresh must stay unset when vulnerability scanning is disabled"); } + /// + /// Verify the Docker instance list masks the user info of a stored endpoint written before the masking existed + /// + /// Task + [TestMethod] + public async Task ApplicationViewServiceDockerInstancesMaskStoredEndpointUserInfoAsync() + { + var options = new DbContextOptionsBuilder().UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbContext = new DockerUpdateGuardDbContext(options); + + await using (dbContext.ConfigureAwait(false)) + { + dbContext.DockerInstances.Add(new DockerInstance + { + Name = "Credentialed", + EndpointUri = "https://user:secret@docker.example.test", + ConnectionKind = DockerConnectionKind.Https, + }); + dbContext.DockerInstances.Add(new DockerInstance + { + Name = "Local", + EndpointUri = "unix:///var/run/docker.sock", + ConnectionKind = DockerConnectionKind.UnixSocket, + }); + await dbContext.SaveChangesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var service = new ApplicationViewService(dbContext, + new ImageReferenceParser(), + CreateOptionsMonitor(), + new SharedBaseImageQueryService(dbContext)); + var dockerInstances = await service.GetDockerInstancesAsync(CancellationToken.None) + .ConfigureAwait(false); + + Assert.AreEqual("https://***@docker.example.test/", + dockerInstances.Single(item => item.Name == "Credentialed").EndpointUri, + "A stored endpoint with user info must be listed with the user info masked"); + Assert.AreEqual("unix:///var/run/docker.sock", + dockerInstances.Single(item => item.Name == "Local").EndpointUri, + "A stored endpoint without user info must be listed unchanged"); + } + } + + /// + /// Verify the Docker instance detail masks the user info of a stored endpoint written before the masking existed + /// + /// Task + [TestMethod] + public async Task ApplicationViewServiceDockerInstanceDetailMasksStoredEndpointUserInfoAsync() + { + var options = new DbContextOptionsBuilder().UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbContext = new DockerUpdateGuardDbContext(options); + + await using (dbContext.ConfigureAwait(false)) + { + dbContext.DockerInstances.Add(new DockerInstance + { + Name = "Credentialed", + EndpointUri = "https://user:secret@docker.example.test", + ConnectionKind = DockerConnectionKind.Https, + }); + await dbContext.SaveChangesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var service = new ApplicationViewService(dbContext, + new ImageReferenceParser(), + CreateOptionsMonitor(), + new SharedBaseImageQueryService(dbContext)); + var instanceId = await dbContext.DockerInstances.Select(entity => entity.Id) + .SingleAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + var detail = await service.GetDockerInstanceDetailAsync(instanceId, CancellationToken.None) + .ConfigureAwait(false); + + Assert.IsNotNull(detail, "The Docker instance detail must be returned"); + Assert.AreEqual("https://***@docker.example.test/", + detail.EndpointUri, + "A stored endpoint with user info must be shown with the user info masked"); + } + } + + /// + /// Verify an already masked stored endpoint is shown unchanged in the list and the detail + /// + /// Task + [TestMethod] + public async Task ApplicationViewServiceDockerInstancesKeepAlreadyMaskedEndpointAsync() + { + var options = new DbContextOptionsBuilder().UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbContext = new DockerUpdateGuardDbContext(options); + + await using (dbContext.ConfigureAwait(false)) + { + dbContext.DockerInstances.Add(new DockerInstance + { + Name = "Masked", + EndpointUri = "https://***@docker.example.test/", + ConnectionKind = DockerConnectionKind.Https, + }); + await dbContext.SaveChangesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var service = new ApplicationViewService(dbContext, + new ImageReferenceParser(), + CreateOptionsMonitor(), + new SharedBaseImageQueryService(dbContext)); + var dockerInstances = await service.GetDockerInstancesAsync(CancellationToken.None) + .ConfigureAwait(false); + var detail = await service.GetDockerInstanceDetailAsync(dockerInstances.Single().Id, CancellationToken.None) + .ConfigureAwait(false); + + Assert.AreEqual("https://***@docker.example.test/", + dockerInstances.Single().EndpointUri, + "An already masked endpoint must be listed unchanged"); + Assert.IsNotNull(detail, "The Docker instance detail must be returned"); + Assert.AreEqual("https://***@docker.example.test/", + detail.EndpointUri, + "An already masked endpoint must be shown unchanged in the detail"); + } + } + #endregion // Methods } \ No newline at end of file diff --git a/src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs b/src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs index 8b9918dc..6ef94741 100644 --- a/src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs @@ -706,5 +706,107 @@ public void DockerInstanceClientCreateHttpClientWhenCertificateValidationEnabled "The Docker HTTP-client factory must not warn about disabled certificate validation when it stays enabled"); } + /// + /// Verify an unsupported endpoint with user info is reported and logged with the user info masked + /// + /// Task + [TestMethod] + public async Task DockerInstanceClientDiscoverContainersAsyncWithUnsupportedEndpointUserInfoMasksUserInfoAsync() + { + var logger = new TestLogger(); + var client = new DockerInstanceClient(logger); + var instanceOptions = new DockerInstanceOptions + { + Name = "Production", + BaseUrl = "ssh://user:secret@docker.example.test", + Enabled = true, + }; + + var result = await client.DiscoverContainersAsync(instanceOptions, CancellationToken.None) + .ConfigureAwait(false); + + var warning = logger.Entries.Single(entry => entry.EventId.Id == 3101); + + Assert.AreEqual(ExternalOperationStatus.Unsupported, + result.Status, + "An unsupported endpoint must return an unsupported result"); + Assert.IsNotNull(result.Message, "An unsupported endpoint must produce a message"); + Assert.Contains("ssh://***@docker.example.test/", + result.Message, + "The result message must name the endpoint with its user info masked"); + Assert.DoesNotContain("secret", result.Message, "The result message must not contain the password"); + Assert.Contains("ssh://***@docker.example.test/", + warning.Message, + "The rejected endpoint log must name the endpoint with its user info masked"); + Assert.DoesNotContain("secret", warning.Message, "The rejected endpoint log must not contain the password"); + } + + /// + /// Verify the other operations report an unsupported endpoint with user info masked + /// + /// Task + [TestMethod] + public async Task DockerInstanceClientOperationsWithUnsupportedEndpointUserInfoMaskUserInfoAsync() + { + var client = new DockerInstanceClient(new TestLogger()); + var instanceOptions = new DockerInstanceOptions + { + Name = "Production", + BaseUrl = "ssh://user:secret@docker.example.test", + Enabled = true, + }; + + var usage = await client.CollectContainerResourceUsageAsync(instanceOptions, CancellationToken.None).ConfigureAwait(false); + var inspect = await client.InspectImageAsync(instanceOptions, "sha256:image", CancellationToken.None).ConfigureAwait(false); + var history = await client.GetImageHistoryAsync(instanceOptions, "sha256:image", CancellationToken.None).ConfigureAwait(false); + var memory = await client.GetHostMemoryTotalAsync(instanceOptions, CancellationToken.None).ConfigureAwait(false); + + Assert.AreEqual(ExternalOperationStatus.Unsupported, usage.Status, "Resource sampling must report an unsupported endpoint"); + Assert.AreEqual(ExternalOperationStatus.Unsupported, inspect.Status, "Image inspection must report an unsupported endpoint"); + Assert.AreEqual(ExternalOperationStatus.Unsupported, history.Status, "Image history must report an unsupported endpoint"); + Assert.AreEqual(ExternalOperationStatus.Unsupported, memory.Status, "Host memory lookup must report an unsupported endpoint"); + Assert.Contains("ssh://***@docker.example.test/", usage.Message ?? string.Empty, "Resource sampling must mask the user info"); + Assert.Contains("ssh://***@docker.example.test/", inspect.Message ?? string.Empty, "Image inspection must mask the user info"); + Assert.Contains("ssh://***@docker.example.test/", history.Message ?? string.Empty, "Image history must mask the user info"); + Assert.Contains("ssh://***@docker.example.test/", memory.Message ?? string.Empty, "Host memory lookup must mask the user info"); + Assert.DoesNotContain("secret", usage.Message ?? string.Empty, "Resource sampling must not contain the password"); + Assert.DoesNotContain("secret", inspect.Message ?? string.Empty, "Image inspection must not contain the password"); + Assert.DoesNotContain("secret", history.Message ?? string.Empty, "Image history must not contain the password"); + Assert.DoesNotContain("secret", memory.Message ?? string.Empty, "Host memory lookup must not contain the password"); + } + + /// + /// Verify a named-pipe endpoint with user info and no pipe name fails with the user info masked + /// + /// Task + [TestMethod] + public async Task DockerInstanceClientDiscoverContainersAsyncWithNamedPipeWithoutPipeNameMasksUserInfoAsync() + { + var logger = new TestLogger(); + var client = new DockerInstanceClient(logger); + var instanceOptions = new DockerInstanceOptions + { + Name = "Production", + BaseUrl = "npipe://user:secret@pipehost/", + Enabled = true, + }; + + var result = await client.DiscoverContainersAsync(instanceOptions, CancellationToken.None) + .ConfigureAwait(false); + + var failure = logger.Entries.Single(entry => entry.EventId.Id == 3104); + + Assert.AreEqual(ExternalOperationStatus.Failed, + result.Status, + "A named-pipe endpoint without a pipe name must fail discovery"); + Assert.IsNotNull(result.Message, "A failed discovery must produce a message"); + Assert.Contains("npipe://***@pipehost/", + result.Message, + "The failure message must name the pipe URI with its user info masked"); + Assert.DoesNotContain("secret", result.Message, "The failure message must not contain the password"); + Assert.IsNotNull(failure.Exception, "The discovery failure must be logged with its exception"); + Assert.DoesNotContain("secret", failure.Exception.Message, "The logged exception must not contain the password"); + } + #endregion // Methods } \ No newline at end of file diff --git a/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs b/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs index 46df8901..0a8d7cdd 100644 --- a/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs +++ b/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs @@ -37,6 +37,11 @@ public RetryProbeHttpMessageHandler(IEnumerable outcomes, HttpS /// public int AttemptCount { get; private set; } + /// + /// URI text of the most recent request the handler received + /// + public string? LastRequestUri { get; private set; } + /// /// Optional Retry-After header value applied to every emitted response /// @@ -50,6 +55,7 @@ public RetryProbeHttpMessageHandler(IEnumerable outcomes, HttpS protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { AttemptCount++; + LastRequestUri = request.RequestUri is { IsAbsoluteUri: false } relativeUri ? relativeUri.OriginalString : request.RequestUri?.AbsoluteUri; var outcome = _outcomes.Count > 0 ? _outcomes.Dequeue() : _fallbackStatusCode; diff --git a/src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs b/src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs index aeca4f0b..2b2bb148 100644 --- a/src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs @@ -180,6 +180,189 @@ await service.SynchronizeConfiguredInstancesAsync(CancellationToken.None) } } + /// + /// Verify the stored endpoint URI of a configured instance carries its user info masked + /// + /// Task + [TestMethod] + public async Task InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresEndpointWithUserInfoMaskedAsync() + { + using (var database = new SqliteTestDatabase()) + { + var dbContext = database.CreateDbContext(); + + await using (dbContext.ConfigureAwait(false)) + { + var options = CreateOptions("https://user:secret@docker.example.test:2376", false, null); + var optionsMonitor = new TestOptionsMonitor(options); + var service = new InstanceDiscoveryService(dbContext, + new TestLogger(), + optionsMonitor); + + await service.SynchronizeConfiguredInstancesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var dockerInstance = await dbContext.DockerInstances.SingleAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + + Assert.AreEqual("https://***@docker.example.test:2376/", + dockerInstance.EndpointUri, + "The stored endpoint URI must carry the user info masked"); + Assert.AreEqual(DockerConnectionKind.Https, + dockerInstance.ConnectionKind, + "Masking the stored endpoint must not change the connection kind derived from the configuration"); + Assert.AreEqual("https://user:secret@docker.example.test:2376", + options.DockerInstances[0].BaseUrl, + "Masking the stored endpoint must leave the configured options untouched"); + } + } + } + + /// + /// Verify the stored Portainer base URL of a configured instance carries its user info masked + /// + /// Task + [TestMethod] + public async Task InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresPortainerBaseUrlMaskedAsync() + { + using (var database = new SqliteTestDatabase()) + { + var dbContext = database.CreateDbContext(); + + await using (dbContext.ConfigureAwait(false)) + { + var options = CreateOptions("unix:///var/run/docker.sock", true, "https://admin:S3cret@portainer.example.test:9443"); + var optionsMonitor = new TestOptionsMonitor(options); + var service = new InstanceDiscoveryService(dbContext, + new TestLogger(), + optionsMonitor); + + await service.SynchronizeConfiguredInstancesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var portainerEndpoint = await dbContext.PortainerEndpoints.SingleAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + + Assert.AreEqual("https://***@portainer.example.test:9443/", + portainerEndpoint.BaseUrl, + "The stored Portainer base URL must carry the user info masked"); + } + } + } + + /// + /// Verify the unsupported connection settings log names the endpoint with its user info masked + /// + /// Task + [TestMethod] + public async Task InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncUnsupportedEndpointLogMasksUserInfoAsync() + { + using (var database = new SqliteTestDatabase()) + { + var dbContext = database.CreateDbContext(); + + await using (dbContext.ConfigureAwait(false)) + { + var logger = new TestLogger(); + var options = CreateOptions("ssh://user:secret@docker.example.test", false, null); + var optionsMonitor = new TestOptionsMonitor(options); + var service = new InstanceDiscoveryService(dbContext, logger, optionsMonitor); + + await service.SynchronizeConfiguredInstancesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var warning = logger.Entries.Single(entry => entry.EventId.Id == 2091); + + Assert.Contains("ssh://***@docker.example.test/", + warning.Message, + "The unsupported connection settings log must name the endpoint with its user info masked"); + Assert.DoesNotContain("secret", warning.Message, "The unsupported connection settings log must not contain the password"); + } + } + } + + /// + /// Verify a stored Portainer base URL is rewritten masked when Portainer is disabled and an already masked one stays + /// + /// Base URL stored before the synchronization + /// Task + [TestMethod] + [DataRow("https://admin:S3cret@portainer.example.test:9443")] + [DataRow("https://***@portainer.example.test:9443/")] + public async Task InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncDisabledPortainerMasksStoredBaseUrlAsync(string storedBaseUrl) + { + using (var database = new SqliteTestDatabase()) + { + var dbContext = database.CreateDbContext(); + + await using (dbContext.ConfigureAwait(false)) + { + dbContext.DockerInstances.Add(new DockerInstance + { + ConnectionKind = DockerConnectionKind.UnixSocket, + EndpointUri = "unix:///var/run/docker.sock", + Name = "Production", + Source = RegistrationSource.ConfigurationFile, + PortainerEndpoint = new PortainerEndpoint + { + Name = "Production Portainer", + BaseUrl = storedBaseUrl, + IsEnabled = true, + }, + }); + + await dbContext.SaveChangesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var optionsMonitor = new TestOptionsMonitor(CreateOptions("unix:///var/run/docker.sock", false, null)); + var service = new InstanceDiscoveryService(dbContext, + new TestLogger(), + optionsMonitor); + + await service.SynchronizeConfiguredInstancesAsync(CancellationToken.None) + .ConfigureAwait(false); + + var portainerEndpoint = await dbContext.PortainerEndpoints.SingleAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + + Assert.AreEqual("https://***@portainer.example.test:9443/", + portainerEndpoint.BaseUrl, + "A stored Portainer base URL must carry its user info masked after synchronization, also when Portainer is disabled"); + Assert.IsFalse(portainerEndpoint.IsEnabled, "A disabled Portainer configuration must disable the stored endpoint"); + } + } + } + + /// + /// Create Docker instance options with a given endpoint and Portainer base URL + /// + /// Docker endpoint URL + /// Whether Portainer should be enabled + /// Portainer base URL + /// Options + private static DockerUpdateGuardOptions CreateOptions(string baseUrl, bool portainerEnabled, string? portainerBaseUrl) + { + return new DockerUpdateGuardOptions + { + DockerInstances = [ + new DockerInstanceOptions + { + BaseUrl = baseUrl, + Enabled = true, + Name = "Production", + Portainer = new PortainerOptions + { + ApiToken = "pat-value", + BaseUrl = portainerBaseUrl, + Enabled = portainerEnabled, + EndpointId = "1", + RequestTimeoutSeconds = 15 + } + }, + ], + }; + } + /// /// Create representative Docker instance options for synchronization tests /// diff --git a/src/Tests/DockerUpdateGuard.Tests/PortainerClientTests.cs b/src/Tests/DockerUpdateGuard.Tests/PortainerClientTests.cs index 104eed58..2ff6b1d1 100644 --- a/src/Tests/DockerUpdateGuard.Tests/PortainerClientTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/PortainerClientTests.cs @@ -779,6 +779,112 @@ public async Task PortainerClientExecuteActionAsyncWhenActionSucceedsLogsExecuti } } + /// + /// Verify a failed login masks the user info of the base URL in the capability message and the logs + /// + /// Task + [TestMethod] + public async Task PortainerClientGetCapabilityAsyncWithUserInfoAndFailedLoginMasksUserInfoAsync() + { + var handler = new SequenceHttpMessageHandler(); + var httpClient = new HttpClient(handler); + var logger = new TestLogger(); + + try + { + handler.AddResponse("https://admin:S3cret@portainer.example.test:9443/api/auth", new HttpResponseMessage(HttpStatusCode.Unauthorized)); + + var client = new PortainerClient(CreateHttpClientFactory(httpClient), logger); + var instanceOptions = CreateInstanceOptions(InstanceName, + options => + { + options.BaseUrl = "https://admin:S3cret@portainer.example.test:9443"; + options.Username = AdminUsername; + options.Password = "wrong-password"; + options.EndpointId = EndpointId; + }); + + var result = await client.GetCapabilityAsync(instanceOptions, CancellationToken.None) + .ConfigureAwait(false); + + var authFailedEntry = logger.Entries.Single(entry => entry.EventId.Id == 3303); + var exceptionEntry = logger.Entries.Single(entry => entry.EventId.Id == 3306); + + Assert.IsNotNull(result.Message, "A failed login must produce a capability message"); + Assert.Contains("https://***@portainer.example.test:9443/", + result.Message, + "The capability message must name the base URL with its user info masked"); + Assert.DoesNotContain("S3cret", result.Message, "The capability message must not contain the password"); + Assert.Contains("https://***@portainer.example.test:9443/", + authFailedEntry.Message, + "The authentication failure log must name the base URL with its user info masked"); + Assert.DoesNotContain("S3cret", authFailedEntry.Message, "The authentication failure log must not contain the password"); + Assert.IsNotNull(exceptionEntry.Exception, "The capability failure must be logged with its exception"); + Assert.Contains("https://***@portainer.example.test:9443/", + exceptionEntry.Exception.Message, + "The logged exception must name the base URL with its user info masked"); + Assert.DoesNotContain("S3cret", exceptionEntry.Exception.Message, "The logged exception must not contain the password"); + } + finally + { + httpClient.Dispose(); + handler.Dispose(); + } + } + + /// + /// Verify a login answer without JWT masks the user info of the base URL in the capability message and the logs + /// + /// Task + [TestMethod] + public async Task PortainerClientGetCapabilityAsyncWithUserInfoAndMissingJwtMasksUserInfoAsync() + { + var handler = new SequenceHttpMessageHandler(); + var httpClient = new HttpClient(handler); + var logger = new TestLogger(); + + try + { + handler.AddJsonResponse("https://admin:S3cret@portainer.example.test:9443/api/auth", "{}"); + + var client = new PortainerClient(CreateHttpClientFactory(httpClient), logger); + var instanceOptions = CreateInstanceOptions(InstanceName, + options => + { + options.BaseUrl = "https://admin:S3cret@portainer.example.test:9443"; + options.Username = AdminUsername; + options.Password = "wrong-password"; + options.EndpointId = EndpointId; + }); + + var result = await client.GetCapabilityAsync(instanceOptions, CancellationToken.None) + .ConfigureAwait(false); + + var authFailedEntry = logger.Entries.Single(entry => entry.EventId.Id == 3303); + var exceptionEntry = logger.Entries.Single(entry => entry.EventId.Id == 3306); + + Assert.IsNotNull(result.Message, "A login answer without JWT must produce a capability message"); + Assert.Contains("https://***@portainer.example.test:9443/", + result.Message, + "The capability message must name the base URL with its user info masked"); + Assert.DoesNotContain("S3cret", result.Message, "The capability message must not contain the password"); + Assert.Contains("https://***@portainer.example.test:9443/", + authFailedEntry.Message, + "The authentication failure log must name the base URL with its user info masked"); + Assert.DoesNotContain("S3cret", authFailedEntry.Message, "The authentication failure log must not contain the password"); + Assert.IsNotNull(exceptionEntry.Exception, "The capability failure must be logged with its exception"); + Assert.Contains("https://***@portainer.example.test:9443/", + exceptionEntry.Exception.Message, + "The logged exception must name the base URL with its user info masked"); + Assert.DoesNotContain("S3cret", exceptionEntry.Exception.Message, "The logged exception must not contain the password"); + } + finally + { + httpClient.Dispose(); + handler.Dispose(); + } + } + /// /// Create Docker instance options configured for Portainer integration tests /// diff --git a/src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs b/src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs index 79553b0d..11cbc952 100644 --- a/src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs @@ -43,15 +43,17 @@ private static TestOptionsMonitor CreateOptions(int re /// Optional list that receives the backoff delays /// Optional request; a GET request is used when null /// Optional jitter provider; no jitter is applied when null + /// Optional logger that captures the handler's log entries; a new logger is used when null /// Resulting response private static async Task SendThroughRetryHandlerAsync(int retryCount, RetryProbeHttpMessageHandler probe, List? capturedDelays = null, HttpRequestMessage? request = null, - Func? nextJitterMilliseconds = null) + Func? nextJitterMilliseconds = null, + TestLogger? logger = null) { var handler = new TransientHttpRetryHandler(CreateOptions(retryCount), - new TestLogger(), + logger ?? new TestLogger(), (delay, cancellationToken) => { capturedDelays?.Add(delay); @@ -421,5 +423,80 @@ public async Task TransientHttpRetryHandlerBuffersAndRetriesRequestWithContentAs "The retry handler must resend the buffered request content on retry"); } + /// + /// Verify the transient status retry log names the request without user info and query string + /// + /// Task + [TestMethod] + public async Task TransientHttpRetryHandlerTransientStatusLogOmitsUserInfoAndQueryAsync() + { + var probe = new RetryProbeHttpMessageHandler([HttpStatusCode.ServiceUnavailable, HttpStatusCode.OK], HttpStatusCode.OK); + var logger = new TestLogger(); + using var request = new HttpRequestMessage(HttpMethod.Get, "https://user:secret@registry.example.test/v2/path?token=x"); + + using var response = await SendThroughRetryHandlerAsync(2, probe, request: request, logger: logger).ConfigureAwait(false); + + var entry = logger.Entries.Single(candidate => candidate.EventId.Id == 3500); + + Assert.Contains("https://***@registry.example.test/v2/path", + entry.Message, + "The transient status retry log must name the request with its user info masked"); + Assert.DoesNotContain("user", entry.Message, "The transient status retry log must not contain the user name"); + Assert.DoesNotContain("secret", entry.Message, "The transient status retry log must not contain the password"); + Assert.DoesNotContain("token=x", entry.Message, "The transient status retry log must not contain the query string"); + Assert.AreEqual("https://user:secret@registry.example.test/v2/path?token=x", + probe.LastRequestUri, + "The request the inner handler receives must keep its user info and query so that connecting is unchanged"); + } + + /// + /// Verify the transient exception retry log names the request without user info and query string + /// + /// Task + [TestMethod] + public async Task TransientHttpRetryHandlerTransientExceptionLogOmitsUserInfoAndQueryAsync() + { + var probe = new RetryProbeHttpMessageHandler([null, HttpStatusCode.OK], HttpStatusCode.OK); + var logger = new TestLogger(); + using var request = new HttpRequestMessage(HttpMethod.Get, "https://user:secret@registry.example.test/v2/path?token=x"); + + using var response = await SendThroughRetryHandlerAsync(2, probe, request: request, logger: logger).ConfigureAwait(false); + + var entry = logger.Entries.Single(candidate => candidate.EventId.Id == 3501); + + Assert.Contains("https://***@registry.example.test/v2/path", + entry.Message, + "The transient exception retry log must name the request with its user info masked"); + Assert.DoesNotContain("user", entry.Message, "The transient exception retry log must not contain the user name"); + Assert.DoesNotContain("secret", entry.Message, "The transient exception retry log must not contain the password"); + Assert.DoesNotContain("token=x", entry.Message, "The transient exception retry log must not contain the query string"); + Assert.AreEqual("https://user:secret@registry.example.test/v2/path?token=x", + probe.LastRequestUri, + "The request the inner handler receives must keep its user info and query so that connecting is unchanged"); + } + + /// + /// Verify the retry log of a relative request URI drops the query string and masks user info + /// + /// Task + [TestMethod] + public async Task TransientHttpRetryHandlerRelativeRequestUriLogOmitsUserInfoAndQueryAsync() + { + var probe = new RetryProbeHttpMessageHandler([HttpStatusCode.ServiceUnavailable, HttpStatusCode.OK], HttpStatusCode.OK); + var logger = new TestLogger(); + using var request = new HttpRequestMessage(HttpMethod.Get, new Uri("/user:secret@host/v2/path?token=x#frag", UriKind.Relative)); + + using var response = await SendThroughRetryHandlerAsync(2, probe, request: request, logger: logger).ConfigureAwait(false); + + var entry = logger.Entries.Single(candidate => candidate.EventId.Id == 3500); + + Assert.Contains("***@host/v2/path", + entry.Message, + "The retry log of a relative request URI must name the target with its user info masked"); + Assert.DoesNotContain("secret", entry.Message, "The retry log must not contain the password of a relative request URI"); + Assert.DoesNotContain("token=x", entry.Message, "The retry log must not contain the query string of a relative request URI"); + Assert.DoesNotContain("frag", entry.Message, "The retry log must not contain the fragment of a relative request URI"); + } + #endregion // Methods } \ No newline at end of file diff --git a/src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs b/src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs new file mode 100644 index 00000000..b157497d --- /dev/null +++ b/src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs @@ -0,0 +1,178 @@ +using DockerUpdateGuard.Infrastructure; + +namespace DockerUpdateGuard.Tests; + +/// +/// Tests for +/// +[TestClass] +public class UrlRedactionTests +{ + #region Methods + + /// + /// Verify a null or empty URL text yields an empty text + /// + [TestMethod] + public void UrlRedactionRedactUserInfoNullOrEmptyReturnsEmpty() + { + Assert.AreEqual(string.Empty, + UrlRedaction.RedactUserInfo((string?)null), + "A null URL text must be returned as an empty text"); + Assert.AreEqual(string.Empty, + UrlRedaction.RedactUserInfo(string.Empty), + "An empty URL text must be returned as an empty text"); + } + + /// + /// Verify a URL text without an at sign is returned unchanged + /// + /// URL text without an at sign + [TestMethod] + [DataRow("unix:///var/run/docker.sock")] + [DataRow("npipe://./pipe/docker_engine")] + [DataRow("https://host.example/path?token=x")] + public void UrlRedactionRedactUserInfoWithoutAtSignReturnsInputUnchanged(string url) + { + Assert.AreEqual(url, + UrlRedaction.RedactUserInfo(url), + "A URL text without an at sign carries no user info and must stay as it is"); + } + + /// + /// Verify an already masked URL text is returned unchanged + /// + [TestMethod] + public void UrlRedactionRedactUserInfoAlreadyMaskedReturnsInputUnchanged() + { + var masked = "https://***@docker.example.test/"; + + Assert.AreEqual(masked, + UrlRedaction.RedactUserInfo(masked), + "Masking an already masked URL text must not change it"); + } + + /// + /// Verify parsed user info is replaced by the mask and neither part of it remains + /// + /// URL text with parsed user info + /// Expected masked URL text + /// Part of the user name that must not remain + /// Part of the password that must not remain + [TestMethod] + [DataRow("https://user:secret@portainer.example:9443/api/auth?x=1", "https://***@portainer.example:9443/api/auth?x=1", "user", "secret")] + [DataRow("https://user@host.example", "https://***@host.example/", "user", "user")] + [DataRow("https://:secret@host.example/", "https://***@host.example/", "secret", "secret")] + [DataRow("tcp://user:secret@docker.example:2376", "tcp://***@docker.example:2376/", "user", "secret")] + [DataRow("https://user:secret@[::1]:2376/", "https://***@[::1]:2376/", "user", "secret")] + [DataRow("https://user%40x:se%3Acret@host.example", "https://***@host.example/", "user", "se%3Acret")] + [DataRow("https://user:secret@host.example/p#frag", "https://***@host.example/p#frag", "user", "secret")] + public void UrlRedactionRedactUserInfoUserAndPasswordMasked(string url, + string expected, + string userPart, + string secretPart) + { + var result = UrlRedaction.RedactUserInfo(url); + + Assert.AreEqual(expected, result, "Parsed user info must be replaced by the mask and the rest of the URL kept"); + Assert.DoesNotContain(userPart, result, "The user name must not remain in the masked URL text"); + Assert.DoesNotContain(secretPart, result, "The password must not remain in the masked URL text"); + } + + /// + /// Verify an at sign the parser does not resolve to user info is masked fail-closed + /// + /// URL text with an at sign and no parsed user info + /// Expected masked URL text + /// Part of the user name that must not remain + /// Part of the password that must not remain + [TestMethod] + [DataRow("https://admin:#Xy7secret@portainer:9443", "https://***@portainer:9443", "admin", "Xy7secret")] + [DataRow("https://admin:?Xy7secret@portainer:9443", "https://***@portainer:9443", "admin", "Xy7secret")] + [DataRow("https://admin:/Xy7secret@portainer:9443", "https://***@portainer:9443", "admin", "Xy7secret")] + [DataRow("https://admin:12/34secret@host", "https://***@host", "admin", "34secret")] + [DataRow("https://admin:12#secret@host", "https://***@host", "admin", "secret")] + [DataRow("https://ad/min:secret@host", "https://***@host", "min", "secret")] + [DataRow("https://admin#:secret@host", "https://***@host", "admin", "secret")] + [DataRow("tcp://admin:#secret@docker:2376", "tcp://***@docker:2376", "admin", "secret")] + [DataRow("https://host.example/@scope/pkg", "https://***@scope/pkg", "host.example", "example")] + public void UrlRedactionRedactUserInfoAtSignWithoutParsedUserInfoMasked(string url, + string expected, + string userPart, + string secretPart) + { + var result = UrlRedaction.RedactUserInfo(url); + + Assert.AreEqual(expected, result, "Everything before the last at sign after the scheme separator must be masked"); + Assert.DoesNotContain(userPart, result, "The user name must not remain in the masked URL text"); + Assert.DoesNotContain(secretPart, result, "The password must not remain in the masked URL text"); + } + + /// + /// Verify URL texts the rebuild rule does not take are masked up to their last at sign + /// + /// Unparsable or irregular URL text with user info + /// Expected masked URL text + /// Part of the user name that must not remain + /// Part of the password that must not remain + [TestMethod] + [DataRow("https://user:p@ss@host.example/path", "https://***@host.example/path", "user", "ss")] + [DataRow("https://user:pa/ss@host.example", "https://***@host.example", "user", "pa/ss")] + [DataRow("npipe://user:secret@./pipe/docker_engine", "npipe://***@./pipe/docker_engine", "user", "secret")] + [DataRow("user:secret@host:2376", "***@host:2376", "user", "secret")] + [DataRow("user:secret@host?x=http://a", "***@host?x=http://a", "user", "secret")] + [DataRow("https://a:b@c:/secret@host", "https://***@host", "a:b", "secret")] + [DataRow("https:\\\\user:secret@host.example", "***@host.example", "user", "secret")] + [DataRow("user:secret://x@host", "***@host", "user", "secret")] + [DataRow("1x://user:secret@host", "***@host", "user", "secret")] + public void UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked(string url, + string expected, + string userPart, + string secretPart) + { + var result = UrlRedaction.RedactUserInfo(url); + + Assert.AreEqual(expected, result, "Everything before the last at sign must be masked, keeping a valid scheme separator"); + Assert.DoesNotContain(userPart, result, "The user name must not remain in the masked URL text"); + Assert.DoesNotContain(secretPart, result, "The password must not remain in the masked URL text"); + } + + /// + /// Verify a null URI yields an empty text + /// + [TestMethod] + public void UrlRedactionRedactUserInfoNullUriReturnsEmpty() + { + Assert.AreEqual(string.Empty, + UrlRedaction.RedactUserInfo((Uri?)null), + "A null URI must be returned as an empty text"); + } + + /// + /// Verify an absolute URI is masked through its absolute URI text + /// + [TestMethod] + public void UrlRedactionRedactUserInfoAbsoluteUriMasked() + { + var result = UrlRedaction.RedactUserInfo(new Uri("https://user:secret@host.example/a")); + + Assert.AreEqual("https://***@host.example/a", + result, + "The user info of an absolute URI must be replaced by the mask"); + } + + /// + /// Verify a relative URI is returned through its original string + /// + [TestMethod] + public void UrlRedactionRedactUserInfoRelativeUriUsesOriginalString() + { + var result = UrlRedaction.RedactUserInfo(new Uri("/v2/library/nginx", UriKind.Relative)); + + Assert.AreEqual("/v2/library/nginx", + result, + "A relative URI has no user info and must be returned as its original string"); + } + + #endregion // Methods +} \ No newline at end of file