From 5f2095186bc1e2a27f69a1a9df32e96cb885f53b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 10:48:40 +0000 Subject: [PATCH 01/16] Start squad record for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 specs/issue-295/log.md diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md new file mode 100644 index 0000000..d2c9016 --- /dev/null +++ b/specs/issue-295/log.md @@ -0,0 +1,8 @@ +# Log: issue #295 + +One line per pipeline step: date and time of day (UTC), step, member, result. Lead decisions and escalations +are quoted in full. + +| Date | Step | Member | Result | +| ---- | ---- | ------ | ------ | +| 2026-10-24 10:48 | 1 Intake | Orchestrator | bug: label bug; branch claude/nice-curie-340hld off main 4ba3473 | From cdea067c3db9c2957268c84e5ad1648f391eaf31 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:00:36 +0000 Subject: [PATCH 02/16] Plan for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- ...5-urls-are-output-with-user-info-masked.md | 52 ++++ docs/decisions/README.md | 1 + specs/issue-295/log.md | 1 + specs/issue-295/plan.md | 253 ++++++++++++++++++ 4 files changed, 307 insertions(+) create mode 100644 docs/decisions/0005-urls-are-output-with-user-info-masked.md create mode 100644 specs/issue-295/plan.md 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 0000000..91ee108 --- /dev/null +++ b/docs/decisions/0005-urls-are-output-with-user-info-masked.md @@ -0,0 +1,52 @@ +# 0005: URLs are output with their user info masked + +- **Status:** Proposed +- **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 URL that the platform's URI parser accepts with user info is output rebuilt from its parts with the user +info replaced by `***`; any other text that contains `@` is masked fail-closed from after its `scheme://` (or from its +start) up to its last `@`; 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. +- Text that only looks like user info (an `@` in an unparseable value) is over-masked; accepted, it fails closed. +- 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 112a143..f6c9aa5 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 | Proposed | 2026-10-24 | diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index d2c9016..aab9b57 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -6,3 +6,4 @@ are quoted in full. | Date | Step | Member | Result | | ---- | ---- | ------ | ------ | | 2026-10-24 10:48 | 1 Intake | Orchestrator | bug: label bug; branch claude/nice-curie-340hld off main 4ba3473 | +| 2026-10-24 11:00 | 2 Plan | squad-lead | DONE: tier security, plan.md + decision 0005; areas: secrets, logging, retry handler, Portainer, Docker (opus/medium · usage pending) | diff --git a/specs/issue-295/plan.md b/specs/issue-295/plan.md new file mode 100644 index 0000000..caebc4d --- /dev/null +++ b/specs/issue-295/plan.md @@ -0,0 +1,253 @@ +# Plan: Credentials embedded in configured URLs leak into logs, error messages and the UI + +Source: Issue #295 +Status: Draft +Tier: security — the change adds a redaction control for secrets in configuration (user info of configured URLs) to logs, messages, stored values and the UI. +Security areas touched: Secrets in configuration; Logging and telemetry of external data; Registry and Docker Hub calls (`TransientHttpRetryHandler`); Container actions (`PortainerClient`); Docker Engine access (`DockerInstanceClient`) + +## Problem / root cause + +Every output site passes the configured URL string (or a URI that still holds its user info) straight into the text +(all paths under `src/DockerUpdateGuard/`): + +- `Infrastructure/TransientHttpRetryHandler.cs:131` — `GetLeftPart(UriPartial.Path)` keeps the user info; used by + events `3500`/`3501`. +- `Docker/DockerInstanceClient.cs:983` (event `3101`) and `:985`, `:1063`, `:1152`, `:1189`, `:1223` (`Unsupported` + messages) use `instanceOptions.BaseUrl`. +- `Docker/DockerInstanceClient.cs:474` — `GetNamedPipeName` throws with `'{pipeUri}'`; **not in the issue** (fact 9). +- `Images/InstanceDiscoveryService.cs:109` (stored `EndpointUri`), `:118` (event `2091`), `:155` (stored Portainer + `BaseUrl`); the disabled-Portainer branch `:131-140` keeps an earlier stored `BaseUrl` (fact 11). +- `Portainer/PortainerClient.cs:166`, `:173` (event `3303`), `:168`, `:175` (exception messages, which become the + capability/action `Message` at `:271`, `:364` and the exception of events `3306`/`3310`). +- `UI/ApplicationViewService.cs:2101`, `:2148` map the stored `EndpointUri` unmasked to the view data. + +## Verified facts + +| # | Claim | Evidence | Status | +| - | ----- | -------- | ------ | +| 1 | `GetLeftPart(UriPartial.Path)` keeps user info; `Uri.UserInfo` of `tcp://user:secret@docker.example:2376` is `user:secret` | Scratch probe (.NET 10.0.402): `https://user:secret@portainer.example:9443/api/auth?x=1` → `https://user:secret@portainer.example:9443/api/auth`; tcp `UserInfo=[user:secret]` | confirmed | +| 2 | Validation accepts URLs with user info, and only parseable absolute URIs | `Configuration/DockerUpdateGuardOptionsValidator.cs:299-305`, `:327-339`, `:362-374` (`Uri.TryCreate` + scheme check only) | confirmed | +| 3 | The unsupported-endpoint branches are practically unreachable in production: the validator admits only `http`, `https`, `tcp`, `unix`, `npipe`, exactly the schemes `TryCreateEngineUri` accepts (`DockerInstanceClient.cs:399-436`); options are validated on start and on every rebuild | `ServiceCollectionExtensions.cs:53-56` (`IValidateOptions` + `ValidateOnStart`); the issue's "the unsupported-endpoint paths log exactly such [unparseable] strings" is therefore refuted for production, the masking there is defense in depth | refuted | +| 4 | Issue reproduction "a transient 5xx from [a Docker instance] logs `… GET https://user:S3cret@docker.example:2376/…`" | The Docker Engine `HttpClient` is built without the retry handler (`DockerInstanceClient.cs:281-311`, `new HttpClient(handler)`); the handler is attached only to Docker Hub, OCI registry, release metadata and Portainer clients (`ServiceCollectionExtensions.cs:72-101`). The retry leak is real for those clients' URLs (Portainer `BaseUrl`, `DockerHub:ApiBaseUrl`, release metadata URLs) | refuted | +| 5 | `PortainerClient` has no production caller today, so the Portainer leak is latent | `grep IPortainerClient` finds only the registration `ServiceCollectionExtensions.cs:104`; `docs/areas/container-actions.md` *Known gaps* (#280) | confirmed | +| 6 | Issue: "rows of instances no longer in configuration, which synchronization does not update" | Synchronization **deletes** those rows (`InstanceDiscoveryService.cs:192-197`) and runs at startup before requests are served (`ApplicationInitializationExtensions.cs:47`); unmasked rows survive only until the first synchronization after the upgrade, or a failed one. Display masking stays as defense in depth | refuted | +| 7 | Neither `DockerInstance.EndpointUri` nor `PortainerEndpoint.BaseUrl` is used for connecting | `grep EndpointUri` / `.BaseUrl`: written only at `InstanceDiscoveryService.cs:109,155`, read only at `ApplicationViewService.cs:2101,2148`; clients use the options | confirmed | +| 8 | `url.full` of HTTP client spans carries no user info: on .NET 9+ OpenTelemetry.Instrumentation.Http 1.20.1 takes the runtime's `System.Net.Http` activity, whose `url.full` is `UriRedactionHelper.GetRedactedUriString` = scheme, host, port, path, query values as `*` | Source read: contrib tag `Instrumentation.Http-1.20.1` `HttpHandlerDiagnosticListener.cs:141-150` (sets tags only below .NET 9); dotnet/runtime `release/10.0` `DiagnosticsHandler.cs:132`, `UriRedactionHelper.cs` (switch `System.Net.Http.DisableUriRedaction` / `DOTNET_SYSTEM_NET_HTTP_DISABLEURIREDACTION` turns it off). The HTTP client factory's log lines use the same helper (`Microsoft.Extensions.Http/src/Logging/LogHelper.cs:73,107`). This settles the issue's `url.full` criterion without a test | confirmed | +| 9 | `npipe://user:secret@pipehost/` passes validation, and discovery fails with a message that contains the user info | Probe: parses, `UserInfo=[user:secret]`, `AbsolutePath=/`; `CreateNamedPipeHttpClient` calls `GetNamedPipeName` eagerly (`DockerInstanceClient.cs:355-360`), which throws at `:474`; caught at the discovery `catch` → `Failed("… failed for '': ")` and event `3104` with the exception | confirmed | +| 10 | Runtime HTTP exception messages never contain the request URI | `System.Net.Http` `Strings.resx` (release/10.0): the only `{0}` near a URI is the scheme in `net_http_unsupported_requesturi_scheme`; connect failures use `host:port` (`ConnectHelper.cs:143`) | confirmed | +| 11 | With Portainer disabled, an existing stored endpoint keeps its earlier `BaseUrl` | `InstanceDiscoveryService.cs:131-140` sets only `IsEnabled`/`UpdatedAtUtc` | confirmed | +| 12 | Parser behavior the helper relies on (probe, .NET 10.0.402): two `@` in the authority (`https://user:p@ss@host/path`), a `/`, `?` or `#` in the password, `npipe://user:secret@./pipe/x`, `unix://user:secret@/var/run/docker.sock` and backslash forms do **not** parse; `user:secret@host:2376` parses as scheme `user` with empty `UserInfo` and path `secret@host:2376`; percent-escaped, IPv6 (`[::1]`), upper-case and whitespace-padded forms parse with `UserInfo` set; `GetComponents(Host \| Port \| PathAndQuery \| Fragment, UriEscaped)` omits default ports, keeps `[::1]:2376`, and gives `unix:///var/run/docker.sock` as `/var/run/docker.sock` | Scratch probe in the session scratchpad | confirmed | +| 13 | The test logger records the formatted message only, not the structured `{BaseUrl}` attribute; the attribute carries the same argument | `src/Tests/DockerUpdateGuard.Tests/Data/TestLogger{TCategoryName}.cs:54-68`; source-generated `LoggerMessage` passes one value to both | confirmed | + +## Acceptance criteria + +The masked form `***` is the constant `UrlRedaction.Mask`. "Contains no secret" means the test's password and user +name strings do not occur in the text (`Assert.DoesNotContain` with a message). + +- [ ] AC1: `UrlRedaction.RedactUserInfo(string?)` returns, for each input, exactly: + + | Input | Output | Rule | + | ----- | ------ | ---- | + | `null`, `""` | `""` | empty | + | `unix:///var/run/docker.sock`, `npipe://./pipe/docker_engine`, `https://host.example/path?token=x` | unchanged | no `@` | + | `https://host.example/@scope/pkg` | unchanged | parsed with authority, no user info | + | `https://user:secret@portainer.example:9443/api/auth?x=1` | `https://***@portainer.example:9443/api/auth?x=1` | parsed with user info | + | `https://user@host.example` | `https://***@host.example/` | parsed with user info | + | `https://:secret@host.example/` | `https://***@host.example/` | parsed with user info | + | `tcp://user:secret@docker.example:2376` | `tcp://***@docker.example:2376/` | parsed with user info | + | `https://user:secret@[::1]:2376/` | `https://***@[::1]:2376/` | parsed with user info | + | `https://user%40x:se%3Acret@host.example` | `https://***@host.example/` | parsed with user info | + | `https://user:secret@host.example/p#frag` | `https://***@host.example/p#frag` | parsed with user info | + | `https://user:p@ss@host.example/path` | `https://***@host.example/path` | fallback | + | `https://user:pa/ss@host.example` | `https://***@host.example` | fallback | + | `npipe://user:secret@./pipe/docker_engine` | `npipe://***@./pipe/docker_engine` | fallback | + | `user:secret@host:2376` | `***@host:2376` | fallback (parsed as scheme `user`, no authority) | + | `user:secret@host?x=http://a` | `***@host?x=http://a` | fallback (`://` only after the `@`) | + +- [ ] AC2: `UrlRedaction.RedactUserInfo(Uri?)` returns `""` for `null`, the AC1 result of `AbsoluteUri` for an + absolute URI (`new Uri("https://user:secret@host.example/a")` → `https://***@host.example/a`) and of + `OriginalString` for a relative one. +- [ ] AC3: A request to `https://user:secret@registry.example.test/v2/path?token=x` that the retry handler retries + logs event `3500` (transient status) and event `3501` (transient exception) with + `https://***@registry.example.test/v2/path` and without `user`, `secret` or `token=x`; the request the inner + handler receives still carries `user:secret` (connecting is unchanged). +- [ ] AC4: With Portainer `BaseUrl` `https://admin:S3cret@portainer.example.test:9443`, user name/password login and a + login answer `401`, `GetCapabilityAsync` returns a `Message` and logs events `3303` and `3306` (message and + exception message) that contain `https://***@portainer.example.test:9443/` and not `S3cret`; the same holds for a + `200` login answer without JWT. +- [ ] AC5: With `BaseUrl` `ssh://user:secret@docker.example.test` and `Enabled = true`, each of + `DiscoverContainersAsync`, `CollectContainerResourceUsageAsync`, `InspectImageAsync`, `GetImageHistoryAsync` and + `GetHostMemoryTotalAsync` returns `Unsupported` with a message containing `ssh://***@docker.example.test/` and no + `secret`; discovery's event `3101` likewise. +- [ ] AC6: With `BaseUrl` `npipe://user:secret@pipehost/`, `DiscoverContainersAsync` returns `Failed` with a message + containing `npipe://***@pipehost/` and no `secret`, and event `3104`'s exception message contains no `secret`. +- [ ] AC7: Synchronization stores `EndpointUri` `https://***@docker.example.test:2376/` for a configured + `https://user:secret@docker.example.test:2376` (connection kind still `Https`, the options' `BaseUrl` unchanged), + stores the Portainer `BaseUrl` masked, logs event `2091` for an enabled `ssh://user:secret@docker.example.test` + masked, and rewrites a stored Portainer `BaseUrl` with user info masked when Portainer is disabled. +- [ ] AC8: `GetDockerInstancesAsync` and `GetDockerInstanceDetailAsync` return `EndpointUri` + `https://***@docker.example.test/` for a stored row whose `EndpointUri` is `https://user:secret@docker.example.test` + (a row written before the change), and `unix:///var/run/docker.sock` unchanged. + +## Tests per criterion + +| AC | Test(s) | Test file | +| -- | ------- | --------- | +| AC1 | `UrlRedactionRedactUserInfoNullOrEmptyReturnsEmpty`, `UrlRedactionRedactUserInfoWithoutAtSignReturnsInputUnchanged` (data rows: unix, npipe, query), `UrlRedactionRedactUserInfoAtSignInPathReturnsInputUnchanged`, `UrlRedactionRedactUserInfoUserAndPasswordMasked` (data rows: the seven "parsed with user info" rows), `UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked` (data rows: the five fallback rows); every masked row also asserts that neither the user name nor the password of its input occurs in the result | `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs` | +| AC2 | `UrlRedactionRedactUserInfoNullUriReturnsEmpty`, `UrlRedactionRedactUserInfoAbsoluteUriMasked`, `UrlRedactionRedactUserInfoRelativeUriUsesOriginalString` | `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs` | +| AC3 | `TransientHttpRetryHandlerTransientStatusLogOmitsUserInfoAndQueryAsync`, `TransientHttpRetryHandlerTransientExceptionLogOmitsUserInfoAndQueryAsync` | `src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs` | +| AC4 | `PortainerClientGetCapabilityAsyncWithUserInfoAndFailedLoginMasksUserInfoAsync`, `PortainerClientGetCapabilityAsyncWithUserInfoAndMissingJwtMasksUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/PortainerClientTests.cs` | +| AC5 | `DockerInstanceClientDiscoverContainersAsyncWithUnsupportedEndpointUserInfoMasksUserInfoAsync`, `DockerInstanceClientOperationsWithUnsupportedEndpointUserInfoMaskUserInfoAsync` (the four other operations) | `src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs` | +| AC6 | `DockerInstanceClientDiscoverContainersAsyncWithNamedPipeWithoutPipeNameMasksUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs` | +| AC7 | `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresEndpointWithUserInfoMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresPortainerBaseUrlMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncUnsupportedEndpointLogMasksUserInfoAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncDisabledPortainerMasksStoredBaseUrlAsync` | `src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs` | +| AC8 | `ApplicationViewServiceDockerInstancesMaskStoredEndpointUserInfoAsync`, `ApplicationViewServiceDockerInstanceDetailMasksStoredEndpointUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs` | + +`RedactUserInfo(null)` is ambiguous between the two overloads: the tests write `RedactUserInfo((string?)null)` and +`RedactUserInfo((Uri?)null)`. + +## Approach + +1. **Helper** `Infrastructure/UrlRedaction.cs`, `internal static class` (pattern of `PaginationLinkGuard`). String + overload, in this order: + 1. `null` or empty → `""`. + 2. No `@` in the input → the input unchanged. + 3. `Uri.TryCreate(input, UriKind.Absolute, …)` succeeds and the trimmed input starts with `{uri.Scheme}://` + (ordinal, case-insensitive), i.e. the parsed URI has an authority: + - `UserInfo` not empty → `$"{uri.Scheme}://{Mask}@{uri.GetComponents(UriComponents.Host | UriComponents.Port | UriComponents.PathAndQuery | UriComponents.Fragment, UriFormat.UriEscaped)}"`; + - `UserInfo` empty → the input unchanged (the `@` lies in path, query or fragment for the parser the clients + connect with). + 4. Otherwise (unparseable, or parsed without an authority): `last` = last index of `@`; `start` = index after the first `://` when + that occurs before `last`, else `0`; return `input[..start] + Mask + input[last..]`. Fail-closed: everything + before the last `@` after the scheme separator is masked. String operations only, no regular expression. + Uri overload: `null` → `""`; absolute → string overload of `AbsoluteUri`; relative → of `OriginalString`. +2. **Retry handler** `GetRequestTarget`: absolute → `UrlRedaction.RedactUserInfo(requestUri.GetLeftPart(UriPartial.Path))` + (query still dropped first); relative → `UrlRedaction.RedactUserInfo(requestUri.OriginalString)`. XML summary + names both. The request itself is untouched. +3. **Output sites**: pass `UrlRedaction.RedactUserInfo(…)` instead of the raw value at `DockerInstanceClient.cs:474` + (Uri overload), `:983`, `:985`, `:1063`, `:1152`, `:1189`, `:1223`; `InstanceDiscoveryService.cs:118`; + `PortainerClient.cs:166`, `:168`, `:173`, `:175` (compute the masked value once per call). Message texts, event + IDs, levels and parameter names (`{BaseUrl}`) stay as they are. +4. **Stored values**: `InstanceDiscoveryService.cs:109` stores `UrlRedaction.RedactUserInfo(configuredInstance.BaseUrl.Trim())`; + `:155` stores `UrlRedaction.RedactUserInfo(configuredInstance.Portainer.BaseUrl?.Trim())`; in the disabled branch + (`:133-137`) also `PortainerEndpoint.BaseUrl = UrlRedaction.RedactUserInfo(PortainerEndpoint.BaseUrl)`. + `MapConnectionKind` keeps reading the options. No migration. +5. **Display**: `ApplicationViewService.cs:2101`, `:2148` map `UrlRedaction.RedactUserInfo(entity.EndpointUri)`. The + pages render the view data as before (no `.razor` change), so the prerendered persistent state is masked too. +6. Layout of output: unchanged except the URL text; no new user-visible element. + +## Options considered + +Weighed in [0005](../../docs/decisions/0005-urls-are-output-with-user-info-masked.md): masking with `***` (chosen), +dropping user info, rejecting it in validation (product decision, breaks working configurations), a log processor +(misses console, messages, database and UI), the issue's anchored regular expression (leaks the part after a second +`@`, misses a `/` in the password), display-only masking (secret stays at rest). One further choice: masking inside +the `LoggerMessage` methods instead of at the call sites — rejected, the exception and result messages need the +masked value too, and one rule per call site is easier to review. + +## Affected projects and types + +| Project | Type / file | Change | +| ------- | ----------- | ------ | +| DockerUpdateGuard | `Infrastructure/UrlRedaction.cs` | new helper | +| DockerUpdateGuard | `Infrastructure/TransientHttpRetryHandler.cs` | `GetRequestTarget` masks user info | +| DockerUpdateGuard | `Docker/DockerInstanceClient.cs` | seven output sites | +| DockerUpdateGuard | `Images/InstanceDiscoveryService.cs` | stored values and event `2091` | +| DockerUpdateGuard | `Portainer/PortainerClient.cs` | event `3303` and two exception messages | +| DockerUpdateGuard | `UI/ApplicationViewService.cs` | two mappings | + +## Signatures (for the Dev's skeleton) + +```csharp +namespace DockerUpdateGuard.Infrastructure; + +internal static class UrlRedaction +{ + public const string Mask = "***"; + public static string RedactUserInfo(string? url); + public static string RedactUserInfo(Uri? uri); +} +``` + +No other signature changes; every other file listed above changes only method bodies. + +## Test files + +- New: `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs`. +- Extended: `TransientHttpRetryHandlerTests.cs`, `PortainerClientTests.cs`, `DockerInstanceClientTests.cs`, + `InstanceDiscoveryServiceTests.cs`, `ApplicationViewServiceTests.cs` (all in `src/Tests/DockerUpdateGuard.Tests/`). + The retry tests need the log entries: `SendThroughRetryHandlerAsync` gains an optional `TestLogger` parameter (the + Tester's change, test code only). +- Existing test call sites of a changed signature: none. `DockerInstanceClientDiscoverContainersAsyncWithUnsupportedEndpointReturnsUnsupportedAndLogsWarningAsync` + asserts `ssh://docker.example.test` in the log; that value has no `@` and stays unchanged. +- Files no unit test can cover: none. + +## Areas + +- Telemetry: *Credentials and external data* — configured URLs and retry lines carry no user info (masked `***`); + HTTP client spans and client-factory log lines carry none by the platform; known gap #295 removed; 0005 linked. +- Web UI: *Secrets* — endpoints shown with user info masked, also for rows stored before; known gap #295 removed. +- Docker instances: stored endpoint URI masked; messages naming an endpoint mask its user info. +- Container actions: stored base URL masked (also when Portainer is disabled); error messages and logs name the base + URL with user info masked. +- Registries: retry log line carries the URL without user info and query. + +## Documentation updates + +All edits by the **Dev**, after the code, checked against it: + +- `docs/areas/telemetry.md` — *Credentials and external data* bullet 3 ("configured base URLs" → with user info + replaced by `***`; "Retry log lines carry the request URL without its user info and query string"); a bullet that + 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`); + remove the #295 *Known gaps* entry; *Related decisions* → 0005. +- `docs/areas/web-ui.md` — *Secrets*: replace "Docker instance endpoints are shown as configured (see *Known gaps*)" + with the masked rule and an example (`https://***@docker.example:2376/`); remove the #295 *Known gaps* entry; + *Related decisions* → 0005. +- `docs/areas/docker-instances.md` — line 26 "(trimmed `BaseUrl`, its user info masked)"; *Limits and errors*: a + message or log entry that names an endpoint shows it with user info masked. +- `docs/areas/container-actions.md` — line 20 masked `BaseUrl`; line 21 disabled endpoint: base URL rewritten masked; + lines 42 and 88: the base URL is named with user info masked. +- `docs/areas/registries.md` — line 166 "the URL without its user info and query". +- `docs/ARCHITECTURE.md` — *Security posture*, *Secrets in configuration* bullet: one sentence that URLs with user info + are logged, stored and shown with it masked, linking [Telemetry](areas/telemetry.md) and record 0005. +- Sweep done: `grep -i "base url|verbatim|as configured|query string|user info"` over `docs/areas`, `ARCHITECTURE.md`, + `README.md`; the hits above are all that the change makes untrue. `README.md` needs no edit. + +## Architecture check + +No guarantee of `.squad/project.md` is weakened. Clients keep returning `ExternalOperationResult` (only message +text changes); layering stays (helper in the host project, next to `PaginationLinkGuard`); no entity or migration +change. `ARCHITECTURE.md` gets the sentence above because the secrets posture gains a rule. + +## Security considerations + +- [x] No limits on parsed input. Masking can lengthen a value by at most 2 characters (user info shorter than `***`) + plus a trailing `/`; `EndpointUri`/`BaseUrl` columns are 512 characters and configuration has no length check + today, so a configured URL of 509–512 characters with one- or two-character user info would now fail the save that + a 513-character URL fails today. Accepted, not worth a new limit. +- [x] Exceptions reaching a user-visible message: the two Portainer `InvalidOperationException`s (text as today with + the masked URL, e.g. `Portainer authentication failed with HTTP 401 for 'https://***@portainer.example.test:9443/'`) + and the named-pipe `InvalidOperationException` (`The Docker named pipe URI 'npipe://***@pipehost/' does not contain + a pipe name`); runtime `HttpRequestException`s carry no URI (fact 10). +- The helper is the guard: the forms the parser accepts or rejects are enumerated in fact 12, and AC1 has a row for + each class (parsed with user info, parsed without, every rejected form seen, no `://`). Any input with `@` that the + parser does not resolve to user info with an authority is masked fail-closed up to its last `@`. +- Scan-run error messages stored before the change are not rewritten (fact 3: the unsupported branches are not + reachable in production; the named-pipe message is, and old rows age out with scan-run retention). + +## Decision records + +- `docs/decisions/0005-urls-are-output-with-user-info-masked.md` (Proposed), indexed. + +## Out of scope / follow-ups + +- Trivy's error excerpt (`TrivyVulnerabilityProvider.cs:305-311`) is passed on as Trivy writes it; whether Trivy echoes + a `TrivyBaseUrl` with user info is unknown. Follow-up issue for the orchestrator: title "Trivy error output may echo + credentials embedded in TrivyBaseUrl", body: "`TrivyVulnerabilityProvider` passes the last line of Trivy's stderr + into logs (`TrivyCommandFailed`) and scan-run messages. If `Vulnerabilities:TrivyBaseUrl` carries user info + (`https://user:secret@trivy:4954`), Trivy may repeat it there. Measure Trivy's output for a failing server call with + such a URL, then decide whether to mask user info in free text with the helper of #295 (`UrlRedaction`), or to + reject user info in `TrivyBaseUrl`." +- `OciRegistryClient.cs:1119,1134` name the token realm URI a registry sends; it is not configured and carries no + configured secret — unchanged. +- Rejecting user info in configuration validation: a product decision (0005, option 3), not proposed here. From dc34c943fa40b294dff077e18e92b06f7cb3426f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:00:54 +0000 Subject: [PATCH 03/16] Log plan usage for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index aab9b57..1a0157a 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -6,4 +6,4 @@ are quoted in full. | Date | Step | Member | Result | | ---- | ---- | ------ | ------ | | 2026-10-24 10:48 | 1 Intake | Orchestrator | bug: label bug; branch claude/nice-curie-340hld off main 4ba3473 | -| 2026-10-24 11:00 | 2 Plan | squad-lead | DONE: tier security, plan.md + decision 0005; areas: secrets, logging, retry handler, Portainer, Docker (opus/medium · usage pending) | +| 2026-10-24 11:00 | 2 Plan | squad-lead | DONE: tier security, plan.md + decision 0005; areas: secrets, logging, retry handler, Portainer, Docker (opus/medium · 203,929 tokens · 68 tool uses · 699 s) | From 7b9a9c8d932043fea3478f5d13921091cd1cbf00 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:04:05 +0000 Subject: [PATCH 04/16] Work in progress: plan revision for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index 1a0157a..12ff1c0 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -7,3 +7,5 @@ are quoted in full. | ---- | ---- | ------ | ------ | | 2026-10-24 10:48 | 1 Intake | Orchestrator | bug: label bug; branch claude/nice-curie-340hld off main 4ba3473 | | 2026-10-24 11:00 | 2 Plan | squad-lead | DONE: tier security, plan.md + decision 0005; areas: secrets, logging, retry handler, Portainer, Docker (opus/medium · 203,929 tokens · 68 tool uses · 699 s) | +| 2026-10-24 11:03 | 2 Plan challenge | squad-devils-advocate | 0 major, 1 minor (fact 12: backslash form parses, masked by fallback) (opus/medium · 78,127 tokens · 18 tool uses · 180 s) | +| 2026-10-24 11:03 | 3 Plan security review | squad-security | CHANGES_REQUIRED: B1 empty-UserInfo branch leaks for passwords starting with # ? / (fail-closed fix); N1 relative-target query; N2 re-redaction idempotent (opus/medium · 67,878 tokens · 10 tool uses · 155 s) | From bbcf35318adb630e353fb50147cf569e750ba1ab Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:05:39 +0000 Subject: [PATCH 05/16] Revise plan for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- ...5-urls-are-output-with-user-info-masked.md | 11 +-- specs/issue-295/plan.md | 72 ++++++++++++++----- 2 files changed, 62 insertions(+), 21 deletions(-) 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 index 91ee108..e13ae38 100644 --- a/docs/decisions/0005-urls-are-output-with-user-info-masked.md +++ b/docs/decisions/0005-urls-are-output-with-user-info-masked.md @@ -33,9 +33,11 @@ areas*). The clients still have to connect with the URL as configured. ## Decision -Option 1. A URL that the platform's URI parser accepts with user info is output rebuilt from its parts with the user -info replaced by `***`; any other text that contains `@` is masked fail-closed from after its `scheme://` (or from its -start) up to its last `@`; text without `@` is output unchanged. The rules and where they apply are specified in +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 @@ -45,7 +47,8 @@ start) up to its last `@`; text without `@` is output unchanged. The rules and w 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. -- Text that only looks like user info (an `@` in an unparseable value) is over-masked; accepted, it fails closed. +- 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 diff --git a/specs/issue-295/plan.md b/specs/issue-295/plan.md index caebc4d..46155e9 100644 --- a/specs/issue-295/plan.md +++ b/specs/issue-295/plan.md @@ -1,7 +1,7 @@ # Plan: Credentials embedded in configured URLs leak into logs, error messages and the UI Source: Issue #295 -Status: Draft +Status: Revised (1) Tier: security — the change adds a redaction control for secrets in configuration (user info of configured URLs) to logs, messages, stored values and the UI. Security areas touched: Secrets in configuration; Logging and telemetry of external data; Registry and Docker Hub calls (`TransientHttpRetryHandler`); Container actions (`PortainerClient`); Docker Engine access (`DockerInstanceClient`) @@ -36,7 +36,7 @@ Every output site passes the configured URL string (or a URI that still holds it | 9 | `npipe://user:secret@pipehost/` passes validation, and discovery fails with a message that contains the user info | Probe: parses, `UserInfo=[user:secret]`, `AbsolutePath=/`; `CreateNamedPipeHttpClient` calls `GetNamedPipeName` eagerly (`DockerInstanceClient.cs:355-360`), which throws at `:474`; caught at the discovery `catch` → `Failed("… failed for '': ")` and event `3104` with the exception | confirmed | | 10 | Runtime HTTP exception messages never contain the request URI | `System.Net.Http` `Strings.resx` (release/10.0): the only `{0}` near a URI is the scheme in `net_http_unsupported_requesturi_scheme`; connect failures use `host:port` (`ConnectHelper.cs:143`) | confirmed | | 11 | With Portainer disabled, an existing stored endpoint keeps its earlier `BaseUrl` | `InstanceDiscoveryService.cs:131-140` sets only `IsEnabled`/`UpdatedAtUtc` | confirmed | -| 12 | Parser behavior the helper relies on (probe, .NET 10.0.402): two `@` in the authority (`https://user:p@ss@host/path`), a `/`, `?` or `#` in the password, `npipe://user:secret@./pipe/x`, `unix://user:secret@/var/run/docker.sock` and backslash forms do **not** parse; `user:secret@host:2376` parses as scheme `user` with empty `UserInfo` and path `secret@host:2376`; percent-escaped, IPv6 (`[::1]`), upper-case and whitespace-padded forms parse with `UserInfo` set; `GetComponents(Host \| Port \| PathAndQuery \| Fragment, UriEscaped)` omits default ports, keeps `[::1]:2376`, and gives `unix:///var/run/docker.sock` as `/var/run/docker.sock` | Scratch probe in the session scratchpad | confirmed | +| 12 | Parser behavior the helper relies on (probe, .NET 10.0.402): (a) two `@` in the authority (`https://user:p@ss@host/path`), a `/` in the password after a non-numeric part (`https://user:pa/ss@host`), `npipe://user:secret@./pipe/x` and `unix://user:secret@/var/run/docker.sock` do **not** parse; (b) the parser ends the authority at the first `/`, `?` or `#`, so a password or user name containing one of them (`https://admin:#Xy7secret@portainer:9443`, `…:?Xy7secret@…`, `…:/Xy7secret@…`, `https://admin:12/34secret@host`, `https://admin:12#secret@host`, `https://ad/min:secret@host`, `https://admin#:secret@host`, `tcp://admin:#secret@docker:2376`) **parses with empty `UserInfo`** and host `admin` — indistinguishable from `https://host.example/@scope/pkg`; (c) `https://a:b@c:/secret@host` parses with `UserInfo` `a:b` and the rest of the secret in the path; (d) backslash forms (`https:\\user:secret@host.example`) parse with `UserInfo` set but do not start with `https://`; (e) `user:secret@host:2376` parses as scheme `user` with empty `UserInfo`; (f) percent-escaped, IPv6 (`[::1]`), upper-case and whitespace-padded forms parse with `UserInfo` set; `GetComponents(Host \| Port \| PathAndQuery \| Fragment, UriEscaped)` omits default ports and keeps `[::1]:2376`; (g) the revised rule (Approach 1) gives the AC1 outputs for every row, and an already masked value is returned unchanged | Scratch probe in the session scratchpad (round 1, re-run in the revision with the rule itself; Security's probe for (b)) | confirmed | | 13 | The test logger records the formatted message only, not the structured `{BaseUrl}` attribute; the attribute carries the same argument | `src/Tests/DockerUpdateGuard.Tests/Data/TestLogger{TCategoryName}.cs:54-68`; source-generated `LoggerMessage` passes one value to both | confirmed | ## Acceptance criteria @@ -50,7 +50,6 @@ name strings do not occur in the text (`Assert.DoesNotContain` with a message). | ----- | ------ | ---- | | `null`, `""` | `""` | empty | | `unix:///var/run/docker.sock`, `npipe://./pipe/docker_engine`, `https://host.example/path?token=x` | unchanged | no `@` | - | `https://host.example/@scope/pkg` | unchanged | parsed with authority, no user info | | `https://user:secret@portainer.example:9443/api/auth?x=1` | `https://***@portainer.example:9443/api/auth?x=1` | parsed with user info | | `https://user@host.example` | `https://***@host.example/` | parsed with user info | | `https://:secret@host.example/` | `https://***@host.example/` | parsed with user info | @@ -63,6 +62,18 @@ name strings do not occur in the text (`Assert.DoesNotContain` with a message). | `npipe://user:secret@./pipe/docker_engine` | `npipe://***@./pipe/docker_engine` | fallback | | `user:secret@host:2376` | `***@host:2376` | fallback (parsed as scheme `user`, no authority) | | `user:secret@host?x=http://a` | `***@host?x=http://a` | fallback (`://` only after the `@`) | + | `https://admin:#Xy7secret@portainer:9443` | `https://***@portainer:9443` | fallback (`@`, no parsed user info) | + | `https://admin:?Xy7secret@portainer:9443` | `https://***@portainer:9443` | fallback (`@`, no parsed user info) | + | `https://admin:/Xy7secret@portainer:9443` | `https://***@portainer:9443` | fallback (`@`, no parsed user info) | + | `https://admin:12/34secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | + | `https://admin:12#secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | + | `https://ad/min:secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | + | `https://admin#:secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | + | `tcp://admin:#secret@docker:2376` | `tcp://***@docker:2376` | fallback (`@`, no parsed user info) | + | `https://host.example/@scope/pkg` | `https://***@scope/pkg` | fallback (`@`, no parsed user info; over-masked, accepted in 0005) | + | `https://a:b@c:/secret@host` | `https://***@host` | fallback (more than one `@`) | + | `https:\\user:secret@host.example` | `***@host.example` | fallback (no `scheme://` prefix) | + | `https://***@docker.example.test/` | `https://***@docker.example.test/` | parsed with user info `***` (idempotent) | - [ ] AC2: `UrlRedaction.RedactUserInfo(Uri?)` returns `""` for `null`, the AC1 result of `AbsoluteUri` for an absolute URI (`new Uri("https://user:secret@host.example/a")` → `https://***@host.example/a`) and of @@ -84,23 +95,25 @@ name strings do not occur in the text (`Assert.DoesNotContain` with a message). - [ ] AC7: Synchronization stores `EndpointUri` `https://***@docker.example.test:2376/` for a configured `https://user:secret@docker.example.test:2376` (connection kind still `Https`, the options' `BaseUrl` unchanged), stores the Portainer `BaseUrl` masked, logs event `2091` for an enabled `ssh://user:secret@docker.example.test` - masked, and rewrites a stored Portainer `BaseUrl` with user info masked when Portainer is disabled. + masked, and rewrites a stored Portainer `BaseUrl` with user info masked when Portainer is disabled (a stored value already + masked, `https://***@portainer.example.test:9443/`, stays exactly as it is). - [ ] AC8: `GetDockerInstancesAsync` and `GetDockerInstanceDetailAsync` return `EndpointUri` `https://***@docker.example.test/` for a stored row whose `EndpointUri` is `https://user:secret@docker.example.test` - (a row written before the change), and `unix:///var/run/docker.sock` unchanged. + (a row written before the change), the already masked `https://***@docker.example.test/` unchanged, and + `unix:///var/run/docker.sock` unchanged. ## Tests per criterion | AC | Test(s) | Test file | | -- | ------- | --------- | -| AC1 | `UrlRedactionRedactUserInfoNullOrEmptyReturnsEmpty`, `UrlRedactionRedactUserInfoWithoutAtSignReturnsInputUnchanged` (data rows: unix, npipe, query), `UrlRedactionRedactUserInfoAtSignInPathReturnsInputUnchanged`, `UrlRedactionRedactUserInfoUserAndPasswordMasked` (data rows: the seven "parsed with user info" rows), `UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked` (data rows: the five fallback rows); every masked row also asserts that neither the user name nor the password of its input occurs in the result | `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs` | +| AC1 | `UrlRedactionRedactUserInfoNullOrEmptyReturnsEmpty`, `UrlRedactionRedactUserInfoWithoutAtSignReturnsInputUnchanged` (data rows: unix, npipe, query), `UrlRedactionRedactUserInfoAtSignWithoutParsedUserInfoMasked` (data rows: the eight `@`-without-parsed-user-info rows and `@scope/pkg`), `UrlRedactionRedactUserInfoAlreadyMaskedReturnsInputUnchanged`, `UrlRedactionRedactUserInfoUserAndPasswordMasked` (data rows: the seven "parsed with user info" rows), `UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked` (data rows: the five fallback rows of round 1, the `a:b@c:/secret@host` row and the backslash row); every masked row also asserts that neither the user name nor any part of the password of its input (`secret`, `Xy7secret`, `34secret`, `min`, `ss`, `cret`) occurs in the result | `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs` | | AC2 | `UrlRedactionRedactUserInfoNullUriReturnsEmpty`, `UrlRedactionRedactUserInfoAbsoluteUriMasked`, `UrlRedactionRedactUserInfoRelativeUriUsesOriginalString` | `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs` | | AC3 | `TransientHttpRetryHandlerTransientStatusLogOmitsUserInfoAndQueryAsync`, `TransientHttpRetryHandlerTransientExceptionLogOmitsUserInfoAndQueryAsync` | `src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs` | | AC4 | `PortainerClientGetCapabilityAsyncWithUserInfoAndFailedLoginMasksUserInfoAsync`, `PortainerClientGetCapabilityAsyncWithUserInfoAndMissingJwtMasksUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/PortainerClientTests.cs` | | AC5 | `DockerInstanceClientDiscoverContainersAsyncWithUnsupportedEndpointUserInfoMasksUserInfoAsync`, `DockerInstanceClientOperationsWithUnsupportedEndpointUserInfoMaskUserInfoAsync` (the four other operations) | `src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs` | | AC6 | `DockerInstanceClientDiscoverContainersAsyncWithNamedPipeWithoutPipeNameMasksUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs` | -| AC7 | `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresEndpointWithUserInfoMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresPortainerBaseUrlMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncUnsupportedEndpointLogMasksUserInfoAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncDisabledPortainerMasksStoredBaseUrlAsync` | `src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs` | -| AC8 | `ApplicationViewServiceDockerInstancesMaskStoredEndpointUserInfoAsync`, `ApplicationViewServiceDockerInstanceDetailMasksStoredEndpointUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs` | +| AC7 | `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresEndpointWithUserInfoMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresPortainerBaseUrlMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncUnsupportedEndpointLogMasksUserInfoAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncDisabledPortainerMasksStoredBaseUrlAsync` (data rows: unmasked and already masked stored value) | `src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs` | +| AC8 | `ApplicationViewServiceDockerInstancesMaskStoredEndpointUserInfoAsync`, `ApplicationViewServiceDockerInstanceDetailMasksStoredEndpointUserInfoAsync`, `ApplicationViewServiceDockerInstancesKeepAlreadyMaskedEndpointAsync` | `src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs` | `RedactUserInfo(null)` is ambiguous between the two overloads: the tests write `RedactUserInfo((string?)null)` and `RedactUserInfo((Uri?)null)`. @@ -111,17 +124,19 @@ name strings do not occur in the text (`Assert.DoesNotContain` with a message). overload, in this order: 1. `null` or empty → `""`. 2. No `@` in the input → the input unchanged. - 3. `Uri.TryCreate(input, UriKind.Absolute, …)` succeeds and the trimmed input starts with `{uri.Scheme}://` - (ordinal, case-insensitive), i.e. the parsed URI has an authority: - - `UserInfo` not empty → `$"{uri.Scheme}://{Mask}@{uri.GetComponents(UriComponents.Host | UriComponents.Port | UriComponents.PathAndQuery | UriComponents.Fragment, UriFormat.UriEscaped)}"`; - - `UserInfo` empty → the input unchanged (the `@` lies in path, query or fragment for the parser the clients - connect with). - 4. Otherwise (unparseable, or parsed without an authority): `last` = last index of `@`; `start` = index after the first `://` when + 3. Rebuild only when **all** hold: the input contains exactly one `@`; `Uri.TryCreate(input, UriKind.Absolute, …)` + succeeds; the trimmed input starts with `{uri.Scheme}://` (ordinal, case-insensitive); `UserInfo` is not empty. + Then return `$"{uri.Scheme}://{Mask}@{uri.GetComponents(UriComponents.Host | UriComponents.Port | UriComponents.PathAndQuery | UriComponents.Fragment, UriFormat.UriEscaped)}"`. + There is no "parsed, no user info → unchanged" branch: the parser cannot tell `https://host/@scope` from + `https://admin:/secret@host` (fact 12 b). + 4. Every other input that contains `@` (unparseable, parsed without user info, without an authority, or with more + than one `@`): `last` = last index of `@`; `start` = index after the first `://` when that occurs before `last`, else `0`; return `input[..start] + Mask + input[last..]`. Fail-closed: everything before the last `@` after the scheme separator is masked. String operations only, no regular expression. Uri overload: `null` → `""`; absolute → string overload of `AbsoluteUri`; relative → of `OriginalString`. 2. **Retry handler** `GetRequestTarget`: absolute → `UrlRedaction.RedactUserInfo(requestUri.GetLeftPart(UriPartial.Path))` - (query still dropped first); relative → `UrlRedaction.RedactUserInfo(requestUri.OriginalString)`. XML summary + (query still dropped first); relative → `OriginalString` cut before its first `?` or `#`, then + `UrlRedaction.RedactUserInfo(…)`, so both branches drop the query as the XML summary says (N1). XML summary names both. The request itself is untouched. 3. **Output sites**: pass `UrlRedaction.RedactUserInfo(…)` instead of the raw value at `DockerInstanceClient.cs:474` (Uri overload), `:983`, `:985`, `:1063`, `:1152`, `:1189`, `:1223`; `InstanceDiscoveryService.cs:118`; @@ -230,8 +245,10 @@ change. `ARCHITECTURE.md` gets the sentence above because the secrets posture ga and the named-pipe `InvalidOperationException` (`The Docker named pipe URI 'npipe://***@pipehost/' does not contain a pipe name`); runtime `HttpRequestException`s carry no URI (fact 10). - The helper is the guard: the forms the parser accepts or rejects are enumerated in fact 12, and AC1 has a row for - each class (parsed with user info, parsed without, every rejected form seen, no `://`). Any input with `@` that the - parser does not resolve to user info with an authority is masked fail-closed up to its last `@`. + each class (parsed with user info, `/`, `?` or `#` in the password or user name, numeric port prefix, more than one + `@`, backslash form, no authority, unparseable, already masked). Only a value with exactly one `@` that the parser + resolves to non-empty user info with a `scheme://` authority is rebuilt; every other value with `@` is masked + fail-closed up to its last `@`, at the price of over-masking an `@` in a path (`https://***@scope/pkg`). - Scan-run error messages stored before the change are not rewritten (fact 3: the unsupported branches are not reachable in production; the named-pipe message is, and old rows age out with scan-run retention). @@ -251,3 +268,24 @@ change. `ARCHITECTURE.md` gets the sentence above because the secrets posture ga - `OciRegistryClient.cs:1119,1134` name the token realm URI a registry sends; it is not configured and carries no configured secret — unchanged. - Rejecting user info in configuration validation: a product decision (0005, option 3), not proposed here. + +## Security review + +- **B1 (blocking) — accepted.** The branch "parsed with authority, empty `UserInfo` → unchanged" is removed (Approach + 1.3). Rebuilding now also requires exactly one `@` (my addition: `https://a:b@c:/secret@host` parses with `UserInfo` + `a:b` and would otherwise keep `secret@host` in the path); everything else with `@` goes to the fallback. AC1 gains + rows for every probe form Security named (including `tcp://admin:#secret@docker:2376`), the `@scope/pkg` row now + expects `https://***@scope/pkg`, and the test is renamed to `UrlRedactionRedactUserInfoAtSignWithoutParsedUserInfoMasked`. + Fact 12, record 0005 (*Decision*, *Consequences*) and *Security considerations* say the same rule. Re-checked the + whole list of accepted forms with the rule itself (fact 12 g): no row leaks a user or password part. +- **N1 — taken.** The relative branch of `GetRequestTarget` cuts `OriginalString` before its first `?` or `#` before + redacting (Approach 2); covered by the existing relative-URI behavior plus the AC3 tests for the absolute branch. +- **N2 — taken.** Re-redacting is idempotent (`***` parses as user info and is rebuilt identically, fact 12 g): AC1 + row, AC7 data row and AC8 test with an already masked stored value. + +## Challenge + +- **Backslash forms (minor) — accepted.** Fact 12 now says `https:\\user:secret@host.example` parses with + `UserInfo` set and, since it does not start with `https://`, goes to the fallback with `***@host.example` (scheme + lost, nothing leaks); AC1 row added to `UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked`. The `scheme://` + prefix condition stays unchanged, keeping `user:secret@host:2376` on the fallback path. From 0c7f7220c34db39c59d5b7936627dc89dbf9be22 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:05:46 +0000 Subject: [PATCH 06/16] Log plan revision for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index 12ff1c0..a8498f2 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -9,3 +9,4 @@ are quoted in full. | 2026-10-24 11:00 | 2 Plan | squad-lead | DONE: tier security, plan.md + decision 0005; areas: secrets, logging, retry handler, Portainer, Docker (opus/medium · 203,929 tokens · 68 tool uses · 699 s) | | 2026-10-24 11:03 | 2 Plan challenge | squad-devils-advocate | 0 major, 1 minor (fact 12: backslash form parses, masked by fallback) (opus/medium · 78,127 tokens · 18 tool uses · 180 s) | | 2026-10-24 11:03 | 3 Plan security review | squad-security | CHANGES_REQUIRED: B1 empty-UserInfo branch leaks for passwords starting with # ? / (fail-closed fix); N1 relative-target query; N2 re-redaction idempotent (opus/medium · 67,878 tokens · 10 tool uses · 155 s) | +| 2026-10-24 11:05 | 2 Plan revise | squad-lead | DONE: B1 accepted (fail-closed rule, exactly-one-@ rebuild), N1, N2, DA minor answered (opus/medium · 19,964 tokens · 8 tool uses · 99 s · continued) | From 6ccc7a581f258da1dbd99161c94ea9c1d587fee2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:07:03 +0000 Subject: [PATCH 07/16] Approve plan for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 1 + specs/issue-295/plan.md | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index a8498f2..b622cac 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -10,3 +10,4 @@ are quoted in full. | 2026-10-24 11:03 | 2 Plan challenge | squad-devils-advocate | 0 major, 1 minor (fact 12: backslash form parses, masked by fallback) (opus/medium · 78,127 tokens · 18 tool uses · 180 s) | | 2026-10-24 11:03 | 3 Plan security review | squad-security | CHANGES_REQUIRED: B1 empty-UserInfo branch leaks for passwords starting with # ? / (fail-closed fix); N1 relative-target query; N2 re-redaction idempotent (opus/medium · 67,878 tokens · 10 tool uses · 155 s) | | 2026-10-24 11:05 | 2 Plan revise | squad-lead | DONE: B1 accepted (fail-closed rule, exactly-one-@ rebuild), N1, N2, DA minor answered (opus/medium · 19,964 tokens · 8 tool uses · 99 s · continued) | +| 2026-10-24 11:07 | 3 Plan security review (delta) | squad-security | APPROVED; N3 (fallback scheme prefix check) appended as Security note (opus/medium · 16,351 tokens · 4 tool uses · 62 s · continued) | diff --git a/specs/issue-295/plan.md b/specs/issue-295/plan.md index 46155e9..352399a 100644 --- a/specs/issue-295/plan.md +++ b/specs/issue-295/plan.md @@ -289,3 +289,14 @@ change. `ARCHITECTURE.md` gets the sentence above because the secrets posture ga `UserInfo` set and, since it does not start with `https://`, goes to the fallback with `***@host.example` (scheme lost, nothing leaks); AC1 row added to `UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked`. The `scheme://` prefix condition stays unchanged, keeping `user:secret@host:2376` on the fallback path. + +## Security notes + +Security's non-blocking finding of the plan review (round 2), verbatim: + +**N3.** The fallback (Approach 1.4) starts masking after the first `://` without checking that the text before it is a valid scheme. So `user:secret://x@host` → `user:secret://***@host` and `https:secret://x@host` → `https:secret://***@host`, and both keep `secret`. Neither can reach an output site: +- Validation admits only http, https, tcp, unix and npipe. +- `https:secret://…` does not parse. +- The retry handler only sees parsed http(s) request URIs. + +This does not hold up the plan. Still, using `start` only when the prefix before `://` matches RFC 3986 `ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )` (and otherwise `0`) would make the "fail-closed" claim hold for any input. It needs one AC1 row, `user:secret://x@host` → `***@host`. A one-line change, which can go into this plan. From ef74a8fdfaf7b90411a3158a38162c0908a5cfb6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:11:56 +0000 Subject: [PATCH 08/16] Skeleton for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 1 + .../Infrastructure/UrlRedaction.cs | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index b622cac..1c29a8c 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -11,3 +11,4 @@ are quoted in full. | 2026-10-24 11:03 | 3 Plan security review | squad-security | CHANGES_REQUIRED: B1 empty-UserInfo branch leaks for passwords starting with # ? / (fail-closed fix); N1 relative-target query; N2 re-redaction idempotent (opus/medium · 67,878 tokens · 10 tool uses · 155 s) | | 2026-10-24 11:05 | 2 Plan revise | squad-lead | DONE: B1 accepted (fail-closed rule, exactly-one-@ rebuild), N1, N2, DA minor answered (opus/medium · 19,964 tokens · 8 tool uses · 99 s · continued) | | 2026-10-24 11:07 | 3 Plan security review (delta) | squad-security | APPROVED; N3 (fallback scheme prefix check) appended as Security note (opus/medium · 16,351 tokens · 4 tool uses · 62 s · continued) | +| 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · usage pending) | diff --git a/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs b/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs new file mode 100644 index 0000000..680a369 --- /dev/null +++ b/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs @@ -0,0 +1,40 @@ +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) + { + throw new InvalidOperationException("Skeleton: not implemented"); + } + + /// + /// 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) + { + throw new InvalidOperationException("Skeleton: not implemented"); + } + + #endregion // Methods +} \ No newline at end of file From 36b7167885379d7d4e89c049f91f66ed8b97bd3a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:12:08 +0000 Subject: [PATCH 09/16] Work in progress: tests first for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index 1c29a8c..741bd11 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -11,4 +11,4 @@ are quoted in full. | 2026-10-24 11:03 | 3 Plan security review | squad-security | CHANGES_REQUIRED: B1 empty-UserInfo branch leaks for passwords starting with # ? / (fail-closed fix); N1 relative-target query; N2 re-redaction idempotent (opus/medium · 67,878 tokens · 10 tool uses · 155 s) | | 2026-10-24 11:05 | 2 Plan revise | squad-lead | DONE: B1 accepted (fail-closed rule, exactly-one-@ rebuild), N1, N2, DA minor answered (opus/medium · 19,964 tokens · 8 tool uses · 99 s · continued) | | 2026-10-24 11:07 | 3 Plan security review (delta) | squad-security | APPROVED; N3 (fallback scheme prefix check) appended as Security note (opus/medium · 16,351 tokens · 4 tool uses · 62 s · continued) | -| 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · usage pending) | +| 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · 82,477 tokens · 12 tool uses · 280 s) | From 9625c674dd14f1dafeb1eb5c7b1bd921ab3aed6f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:17:50 +0000 Subject: [PATCH 10/16] Tests first for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 1 + .../ApplicationViewServiceTests.cs | 127 ++++++++++++ .../DockerInstanceClientTests.cs | 102 ++++++++++ .../Helper/RetryProbeHttpMessageHandler.cs | 6 + .../InstanceDiscoveryServiceTests.cs | 183 ++++++++++++++++++ .../PortainerClientTests.cs | 106 ++++++++++ .../TransientHttpRetryHandlerTests.cs | 58 +++++- .../UrlRedactionTests.cs | 177 +++++++++++++++++ 8 files changed, 758 insertions(+), 2 deletions(-) create mode 100644 src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index 741bd11..d13f96c 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -12,3 +12,4 @@ are quoted in full. | 2026-10-24 11:05 | 2 Plan revise | squad-lead | DONE: B1 accepted (fail-closed rule, exactly-one-@ rebuild), N1, N2, DA minor answered (opus/medium · 19,964 tokens · 8 tool uses · 99 s · continued) | | 2026-10-24 11:07 | 3 Plan security review (delta) | squad-security | APPROVED; N3 (fallback scheme prefix check) appended as Security note (opus/medium · 16,351 tokens · 4 tool uses · 62 s · continued) | | 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · 82,477 tokens · 12 tool uses · 280 s) | +| 2026-10-24 11:17 | 5 Tests first | squad-tester | DONE: 45 of 134 tests fail on skeleton, 89 pass; deferred: ApplicationViewService...KeepAlreadyMaskedEndpointAsync, already-masked row of DisabledPortainer test (sonnet/medium · 156,772 tokens · 30 tool uses · 312 s) | diff --git a/src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs b/src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs index 49a6767..20cf693 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 8b9918d..6ef9474 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 46df890..8d38d90 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; } + /// + /// Absolute URI 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?.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 aeca4f0..2b2bb14 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 104eed5..2ff6b1d 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 79553b0..5e8342c 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,57 @@ 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"); + } + #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 0000000..b15e7df --- /dev/null +++ b/src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs @@ -0,0 +1,177 @@ +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")] + 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 From 565da5d52cbb2c04d1d1d9a53ba8c7d287acb78b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:21:09 +0000 Subject: [PATCH 11/16] Implement URL user-info redaction for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- docs/ARCHITECTURE.md | 4 +- docs/areas/container-actions.md | 9 +-- docs/areas/docker-instances.md | 4 +- docs/areas/registries.md | 2 +- docs/areas/telemetry.md | 13 ++-- docs/areas/web-ui.md | 7 +- specs/issue-295/log.md | 1 + .../Docker/DockerInstanceClient.cs | 14 ++-- .../Images/InstanceDiscoveryService.cs | 8 ++- .../TransientHttpRetryHandler.cs | 7 +- .../Infrastructure/UrlRedaction.cs | 72 ++++++++++++++++++- .../Portainer/PortainerClient.cs | 10 +-- .../UI/ApplicationViewService.cs | 5 +- 13 files changed, 119 insertions(+), 37 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index db205b0..a149780 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 2b2118b..a9a1728 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 f1d9314..2a31c75 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 5cd8727..ea289a2 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 2a3fc56..a448705 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 c95b674..95abab9 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/specs/issue-295/log.md b/specs/issue-295/log.md index d13f96c..0dc5f41 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -13,3 +13,4 @@ are quoted in full. | 2026-10-24 11:07 | 3 Plan security review (delta) | squad-security | APPROVED; N3 (fallback scheme prefix check) appended as Security note (opus/medium · 16,351 tokens · 4 tool uses · 62 s · continued) | | 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · 82,477 tokens · 12 tool uses · 280 s) | | 2026-10-24 11:17 | 5 Tests first | squad-tester | DONE: 45 of 134 tests fail on skeleton, 89 pass; deferred: ApplicationViewService...KeepAlreadyMaskedEndpointAsync, already-masked row of DisabledPortainer test (sonnet/medium · 156,772 tokens · 30 tool uses · 312 s) | +| 2026-10-24 11:21 | 6 Implement | squad-dev | DONE: all 45 tests green; coverage 92.5% new/84.5% overall; uncovered: relative branch GetRequestTarget, IsValidScheme false return (sonnet/medium · usage pending) | diff --git a/src/DockerUpdateGuard/Docker/DockerInstanceClient.cs b/src/DockerUpdateGuard/Docker/DockerInstanceClient.cs index ab5a615..539f8f6 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 921c01d..097a4d5 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 f96f074..2994e9f 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 index 680a369..320ac4b 100644 --- a/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs +++ b/src/DockerUpdateGuard/Infrastructure/UrlRedaction.cs @@ -23,7 +23,39 @@ internal static class UrlRedaction /// URL text with masked user info, empty for null or empty input public static string RedactUserInfo(string? url) { - throw new InvalidOperationException("Skeleton: not implemented"); + 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)); } /// @@ -33,8 +65,44 @@ public static string RedactUserInfo(string? url) /// URI text with masked user info, empty for null public static string RedactUserInfo(Uri? uri) { - throw new InvalidOperationException("Skeleton: not implemented"); + 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 7f5d627..02b8493 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 b34dd4a..1af6e6d 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), From c148ef4f45152e82d2849e4b2ffd20b3dd62b2ca Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:21:24 +0000 Subject: [PATCH 12/16] Work in progress: coverage pass for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index 0dc5f41..5ec6b8d 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -13,4 +13,4 @@ are quoted in full. | 2026-10-24 11:07 | 3 Plan security review (delta) | squad-security | APPROVED; N3 (fallback scheme prefix check) appended as Security note (opus/medium · 16,351 tokens · 4 tool uses · 62 s · continued) | | 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · 82,477 tokens · 12 tool uses · 280 s) | | 2026-10-24 11:17 | 5 Tests first | squad-tester | DONE: 45 of 134 tests fail on skeleton, 89 pass; deferred: ApplicationViewService...KeepAlreadyMaskedEndpointAsync, already-masked row of DisabledPortainer test (sonnet/medium · 156,772 tokens · 30 tool uses · 312 s) | -| 2026-10-24 11:21 | 6 Implement | squad-dev | DONE: all 45 tests green; coverage 92.5% new/84.5% overall; uncovered: relative branch GetRequestTarget, IsValidScheme false return (sonnet/medium · usage pending) | +| 2026-10-24 11:21 | 6 Implement | squad-dev | DONE: all 45 tests green; coverage 92.5% new/84.5% overall; uncovered: relative branch GetRequestTarget, IsValidScheme false return (sonnet/medium · 102,347 tokens · 13 tool uses · 193 s) | From f1fe32743a22658965c8796b967668a7d710f5a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:24:00 +0000 Subject: [PATCH 13/16] Cover remaining redaction branches for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- .../Helper/RetryProbeHttpMessageHandler.cs | 4 ++-- .../TransientHttpRetryHandlerTests.cs | 23 +++++++++++++++++++ .../UrlRedactionTests.cs | 1 + 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs b/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs index 8d38d90..0a8d7cd 100644 --- a/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs +++ b/src/Tests/DockerUpdateGuard.Tests/Helper/RetryProbeHttpMessageHandler.cs @@ -38,7 +38,7 @@ public RetryProbeHttpMessageHandler(IEnumerable outcomes, HttpS public int AttemptCount { get; private set; } /// - /// Absolute URI of the most recent request the handler received + /// URI text of the most recent request the handler received /// public string? LastRequestUri { get; private set; } @@ -55,7 +55,7 @@ public RetryProbeHttpMessageHandler(IEnumerable outcomes, HttpS protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { AttemptCount++; - LastRequestUri = request.RequestUri?.AbsoluteUri; + 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/TransientHttpRetryHandlerTests.cs b/src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs index 5e8342c..11cbc95 100644 --- a/src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs @@ -475,5 +475,28 @@ public async Task TransientHttpRetryHandlerTransientExceptionLogOmitsUserInfoAnd "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 index b15e7df..b157497 100644 --- a/src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs @@ -124,6 +124,7 @@ public void UrlRedactionRedactUserInfoAtSignWithoutParsedUserInfoMasked(string u [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, From 8d47770eb01a375e658e25f6590a78370f55517e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:26:00 +0000 Subject: [PATCH 14/16] Code check for issue 295 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md index 5ec6b8d..f38cfad 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -14,3 +14,5 @@ are quoted in full. | 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · 82,477 tokens · 12 tool uses · 280 s) | | 2026-10-24 11:17 | 5 Tests first | squad-tester | DONE: 45 of 134 tests fail on skeleton, 89 pass; deferred: ApplicationViewService...KeepAlreadyMaskedEndpointAsync, already-masked row of DisabledPortainer test (sonnet/medium · 156,772 tokens · 30 tool uses · 312 s) | | 2026-10-24 11:21 | 6 Implement | squad-dev | DONE: all 45 tests green; coverage 92.5% new/84.5% overall; uncovered: relative branch GetRequestTarget, IsValidScheme false return (sonnet/medium · 102,347 tokens · 13 tool uses · 193 s) | +| 2026-10-24 11:24 | 6 Coverage | squad-tester | DONE: relative-URI retry test, 1x:// scheme row; deferred AC8 test green (sonnet/medium · 6,340 tokens · 5 tool uses · 159 s · continued) | +| 2026-10-24 11:26 | 7 Code check | Orchestrator | Format check, Build, Test 570+21 green, Coverage gate PASS (new 100%, overall 84.5%), Analyzer gate PASS, scope-check PASS; Sonar gate NOT CHECKED (no SONAR_TOKEN) -> step 11 | From 29d480363abe136e86fa2fa5dbc1aa3d4eef9725 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:29:36 +0000 Subject: [PATCH 15/16] Accept decision record 0005 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- docs/decisions/0005-urls-are-output-with-user-info-masked.md | 2 +- docs/decisions/README.md | 2 +- specs/issue-295/log.md | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) 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 index e13ae38..b991081 100644 --- a/docs/decisions/0005-urls-are-output-with-user-info-masked.md +++ b/docs/decisions/0005-urls-are-output-with-user-info-masked.md @@ -1,6 +1,6 @@ # 0005: URLs are output with their user info masked -- **Status:** Proposed +- **Status:** Accepted - **Date:** 2026-10-24 - **Area:** Telemetry - **Source:** Issue #295 diff --git a/docs/decisions/README.md b/docs/decisions/README.md index f6c9aa5..e5ca06b 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -57,5 +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 | Proposed | 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/specs/issue-295/log.md b/specs/issue-295/log.md index f38cfad..ace2952 100644 --- a/specs/issue-295/log.md +++ b/specs/issue-295/log.md @@ -16,3 +16,6 @@ are quoted in full. | 2026-10-24 11:21 | 6 Implement | squad-dev | DONE: all 45 tests green; coverage 92.5% new/84.5% overall; uncovered: relative branch GetRequestTarget, IsValidScheme false return (sonnet/medium · 102,347 tokens · 13 tool uses · 193 s) | | 2026-10-24 11:24 | 6 Coverage | squad-tester | DONE: relative-URI retry test, 1x:// scheme row; deferred AC8 test green (sonnet/medium · 6,340 tokens · 5 tool uses · 159 s · continued) | | 2026-10-24 11:26 | 7 Code check | Orchestrator | Format check, Build, Test 570+21 green, Coverage gate PASS (new 100%, overall 84.5%), Analyzer gate PASS, scope-check PASS; Sonar gate NOT CHECKED (no SONAR_TOKEN) -> step 11 | +| 2026-10-24 11:28 | 8 Review round 1 | squad-reviewer | APPROVE: no blocking, no non-blocking findings (2 style notes not reported: GetRequestTarget summary, region order) (opus/medium · 84,543 tokens · 18 tool uses · 124 s) | +| 2026-10-24 11:28 | 8 Security diff review | squad-security | APPROVED; non-blocking: Trivy CLI error output not masked (out of scope per 0005) -> follow-up issue as planned (opus/medium · 68,797 tokens · 12 tool uses · 127 s) | +| 2026-10-24 11:28 | 9 PR approval | Orchestrator | approved by checklist; record 0005 Accepted; follow-up issue #304 (Trivy output) | From 061cc147d2d83f2484cbb5b9a10273672ad192e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 11:29:55 +0000 Subject: [PATCH 16/16] Remove squad working record Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019unkFPP5TCRVosCsDJtYGB --- specs/issue-295/log.md | 21 --- specs/issue-295/plan.md | 302 ---------------------------------------- 2 files changed, 323 deletions(-) delete mode 100644 specs/issue-295/log.md delete mode 100644 specs/issue-295/plan.md diff --git a/specs/issue-295/log.md b/specs/issue-295/log.md deleted file mode 100644 index ace2952..0000000 --- a/specs/issue-295/log.md +++ /dev/null @@ -1,21 +0,0 @@ -# Log: issue #295 - -One line per pipeline step: date and time of day (UTC), step, member, result. Lead decisions and escalations -are quoted in full. - -| Date | Step | Member | Result | -| ---- | ---- | ------ | ------ | -| 2026-10-24 10:48 | 1 Intake | Orchestrator | bug: label bug; branch claude/nice-curie-340hld off main 4ba3473 | -| 2026-10-24 11:00 | 2 Plan | squad-lead | DONE: tier security, plan.md + decision 0005; areas: secrets, logging, retry handler, Portainer, Docker (opus/medium · 203,929 tokens · 68 tool uses · 699 s) | -| 2026-10-24 11:03 | 2 Plan challenge | squad-devils-advocate | 0 major, 1 minor (fact 12: backslash form parses, masked by fallback) (opus/medium · 78,127 tokens · 18 tool uses · 180 s) | -| 2026-10-24 11:03 | 3 Plan security review | squad-security | CHANGES_REQUIRED: B1 empty-UserInfo branch leaks for passwords starting with # ? / (fail-closed fix); N1 relative-target query; N2 re-redaction idempotent (opus/medium · 67,878 tokens · 10 tool uses · 155 s) | -| 2026-10-24 11:05 | 2 Plan revise | squad-lead | DONE: B1 accepted (fail-closed rule, exactly-one-@ rebuild), N1, N2, DA minor answered (opus/medium · 19,964 tokens · 8 tool uses · 99 s · continued) | -| 2026-10-24 11:07 | 3 Plan security review (delta) | squad-security | APPROVED; N3 (fallback scheme prefix check) appended as Security note (opus/medium · 16,351 tokens · 4 tool uses · 62 s · continued) | -| 2026-10-24 11:11 | 4 Skeleton | squad-dev | DONE: UrlRedaction.cs stubs (InvalidOperationException instead of NotImplementedException: RH2003 is an error in Release); build 0 warnings (sonnet/medium · 82,477 tokens · 12 tool uses · 280 s) | -| 2026-10-24 11:17 | 5 Tests first | squad-tester | DONE: 45 of 134 tests fail on skeleton, 89 pass; deferred: ApplicationViewService...KeepAlreadyMaskedEndpointAsync, already-masked row of DisabledPortainer test (sonnet/medium · 156,772 tokens · 30 tool uses · 312 s) | -| 2026-10-24 11:21 | 6 Implement | squad-dev | DONE: all 45 tests green; coverage 92.5% new/84.5% overall; uncovered: relative branch GetRequestTarget, IsValidScheme false return (sonnet/medium · 102,347 tokens · 13 tool uses · 193 s) | -| 2026-10-24 11:24 | 6 Coverage | squad-tester | DONE: relative-URI retry test, 1x:// scheme row; deferred AC8 test green (sonnet/medium · 6,340 tokens · 5 tool uses · 159 s · continued) | -| 2026-10-24 11:26 | 7 Code check | Orchestrator | Format check, Build, Test 570+21 green, Coverage gate PASS (new 100%, overall 84.5%), Analyzer gate PASS, scope-check PASS; Sonar gate NOT CHECKED (no SONAR_TOKEN) -> step 11 | -| 2026-10-24 11:28 | 8 Review round 1 | squad-reviewer | APPROVE: no blocking, no non-blocking findings (2 style notes not reported: GetRequestTarget summary, region order) (opus/medium · 84,543 tokens · 18 tool uses · 124 s) | -| 2026-10-24 11:28 | 8 Security diff review | squad-security | APPROVED; non-blocking: Trivy CLI error output not masked (out of scope per 0005) -> follow-up issue as planned (opus/medium · 68,797 tokens · 12 tool uses · 127 s) | -| 2026-10-24 11:28 | 9 PR approval | Orchestrator | approved by checklist; record 0005 Accepted; follow-up issue #304 (Trivy output) | diff --git a/specs/issue-295/plan.md b/specs/issue-295/plan.md deleted file mode 100644 index 352399a..0000000 --- a/specs/issue-295/plan.md +++ /dev/null @@ -1,302 +0,0 @@ -# Plan: Credentials embedded in configured URLs leak into logs, error messages and the UI - -Source: Issue #295 -Status: Revised (1) -Tier: security — the change adds a redaction control for secrets in configuration (user info of configured URLs) to logs, messages, stored values and the UI. -Security areas touched: Secrets in configuration; Logging and telemetry of external data; Registry and Docker Hub calls (`TransientHttpRetryHandler`); Container actions (`PortainerClient`); Docker Engine access (`DockerInstanceClient`) - -## Problem / root cause - -Every output site passes the configured URL string (or a URI that still holds its user info) straight into the text -(all paths under `src/DockerUpdateGuard/`): - -- `Infrastructure/TransientHttpRetryHandler.cs:131` — `GetLeftPart(UriPartial.Path)` keeps the user info; used by - events `3500`/`3501`. -- `Docker/DockerInstanceClient.cs:983` (event `3101`) and `:985`, `:1063`, `:1152`, `:1189`, `:1223` (`Unsupported` - messages) use `instanceOptions.BaseUrl`. -- `Docker/DockerInstanceClient.cs:474` — `GetNamedPipeName` throws with `'{pipeUri}'`; **not in the issue** (fact 9). -- `Images/InstanceDiscoveryService.cs:109` (stored `EndpointUri`), `:118` (event `2091`), `:155` (stored Portainer - `BaseUrl`); the disabled-Portainer branch `:131-140` keeps an earlier stored `BaseUrl` (fact 11). -- `Portainer/PortainerClient.cs:166`, `:173` (event `3303`), `:168`, `:175` (exception messages, which become the - capability/action `Message` at `:271`, `:364` and the exception of events `3306`/`3310`). -- `UI/ApplicationViewService.cs:2101`, `:2148` map the stored `EndpointUri` unmasked to the view data. - -## Verified facts - -| # | Claim | Evidence | Status | -| - | ----- | -------- | ------ | -| 1 | `GetLeftPart(UriPartial.Path)` keeps user info; `Uri.UserInfo` of `tcp://user:secret@docker.example:2376` is `user:secret` | Scratch probe (.NET 10.0.402): `https://user:secret@portainer.example:9443/api/auth?x=1` → `https://user:secret@portainer.example:9443/api/auth`; tcp `UserInfo=[user:secret]` | confirmed | -| 2 | Validation accepts URLs with user info, and only parseable absolute URIs | `Configuration/DockerUpdateGuardOptionsValidator.cs:299-305`, `:327-339`, `:362-374` (`Uri.TryCreate` + scheme check only) | confirmed | -| 3 | The unsupported-endpoint branches are practically unreachable in production: the validator admits only `http`, `https`, `tcp`, `unix`, `npipe`, exactly the schemes `TryCreateEngineUri` accepts (`DockerInstanceClient.cs:399-436`); options are validated on start and on every rebuild | `ServiceCollectionExtensions.cs:53-56` (`IValidateOptions` + `ValidateOnStart`); the issue's "the unsupported-endpoint paths log exactly such [unparseable] strings" is therefore refuted for production, the masking there is defense in depth | refuted | -| 4 | Issue reproduction "a transient 5xx from [a Docker instance] logs `… GET https://user:S3cret@docker.example:2376/…`" | The Docker Engine `HttpClient` is built without the retry handler (`DockerInstanceClient.cs:281-311`, `new HttpClient(handler)`); the handler is attached only to Docker Hub, OCI registry, release metadata and Portainer clients (`ServiceCollectionExtensions.cs:72-101`). The retry leak is real for those clients' URLs (Portainer `BaseUrl`, `DockerHub:ApiBaseUrl`, release metadata URLs) | refuted | -| 5 | `PortainerClient` has no production caller today, so the Portainer leak is latent | `grep IPortainerClient` finds only the registration `ServiceCollectionExtensions.cs:104`; `docs/areas/container-actions.md` *Known gaps* (#280) | confirmed | -| 6 | Issue: "rows of instances no longer in configuration, which synchronization does not update" | Synchronization **deletes** those rows (`InstanceDiscoveryService.cs:192-197`) and runs at startup before requests are served (`ApplicationInitializationExtensions.cs:47`); unmasked rows survive only until the first synchronization after the upgrade, or a failed one. Display masking stays as defense in depth | refuted | -| 7 | Neither `DockerInstance.EndpointUri` nor `PortainerEndpoint.BaseUrl` is used for connecting | `grep EndpointUri` / `.BaseUrl`: written only at `InstanceDiscoveryService.cs:109,155`, read only at `ApplicationViewService.cs:2101,2148`; clients use the options | confirmed | -| 8 | `url.full` of HTTP client spans carries no user info: on .NET 9+ OpenTelemetry.Instrumentation.Http 1.20.1 takes the runtime's `System.Net.Http` activity, whose `url.full` is `UriRedactionHelper.GetRedactedUriString` = scheme, host, port, path, query values as `*` | Source read: contrib tag `Instrumentation.Http-1.20.1` `HttpHandlerDiagnosticListener.cs:141-150` (sets tags only below .NET 9); dotnet/runtime `release/10.0` `DiagnosticsHandler.cs:132`, `UriRedactionHelper.cs` (switch `System.Net.Http.DisableUriRedaction` / `DOTNET_SYSTEM_NET_HTTP_DISABLEURIREDACTION` turns it off). The HTTP client factory's log lines use the same helper (`Microsoft.Extensions.Http/src/Logging/LogHelper.cs:73,107`). This settles the issue's `url.full` criterion without a test | confirmed | -| 9 | `npipe://user:secret@pipehost/` passes validation, and discovery fails with a message that contains the user info | Probe: parses, `UserInfo=[user:secret]`, `AbsolutePath=/`; `CreateNamedPipeHttpClient` calls `GetNamedPipeName` eagerly (`DockerInstanceClient.cs:355-360`), which throws at `:474`; caught at the discovery `catch` → `Failed("… failed for '': ")` and event `3104` with the exception | confirmed | -| 10 | Runtime HTTP exception messages never contain the request URI | `System.Net.Http` `Strings.resx` (release/10.0): the only `{0}` near a URI is the scheme in `net_http_unsupported_requesturi_scheme`; connect failures use `host:port` (`ConnectHelper.cs:143`) | confirmed | -| 11 | With Portainer disabled, an existing stored endpoint keeps its earlier `BaseUrl` | `InstanceDiscoveryService.cs:131-140` sets only `IsEnabled`/`UpdatedAtUtc` | confirmed | -| 12 | Parser behavior the helper relies on (probe, .NET 10.0.402): (a) two `@` in the authority (`https://user:p@ss@host/path`), a `/` in the password after a non-numeric part (`https://user:pa/ss@host`), `npipe://user:secret@./pipe/x` and `unix://user:secret@/var/run/docker.sock` do **not** parse; (b) the parser ends the authority at the first `/`, `?` or `#`, so a password or user name containing one of them (`https://admin:#Xy7secret@portainer:9443`, `…:?Xy7secret@…`, `…:/Xy7secret@…`, `https://admin:12/34secret@host`, `https://admin:12#secret@host`, `https://ad/min:secret@host`, `https://admin#:secret@host`, `tcp://admin:#secret@docker:2376`) **parses with empty `UserInfo`** and host `admin` — indistinguishable from `https://host.example/@scope/pkg`; (c) `https://a:b@c:/secret@host` parses with `UserInfo` `a:b` and the rest of the secret in the path; (d) backslash forms (`https:\\user:secret@host.example`) parse with `UserInfo` set but do not start with `https://`; (e) `user:secret@host:2376` parses as scheme `user` with empty `UserInfo`; (f) percent-escaped, IPv6 (`[::1]`), upper-case and whitespace-padded forms parse with `UserInfo` set; `GetComponents(Host \| Port \| PathAndQuery \| Fragment, UriEscaped)` omits default ports and keeps `[::1]:2376`; (g) the revised rule (Approach 1) gives the AC1 outputs for every row, and an already masked value is returned unchanged | Scratch probe in the session scratchpad (round 1, re-run in the revision with the rule itself; Security's probe for (b)) | confirmed | -| 13 | The test logger records the formatted message only, not the structured `{BaseUrl}` attribute; the attribute carries the same argument | `src/Tests/DockerUpdateGuard.Tests/Data/TestLogger{TCategoryName}.cs:54-68`; source-generated `LoggerMessage` passes one value to both | confirmed | - -## Acceptance criteria - -The masked form `***` is the constant `UrlRedaction.Mask`. "Contains no secret" means the test's password and user -name strings do not occur in the text (`Assert.DoesNotContain` with a message). - -- [ ] AC1: `UrlRedaction.RedactUserInfo(string?)` returns, for each input, exactly: - - | Input | Output | Rule | - | ----- | ------ | ---- | - | `null`, `""` | `""` | empty | - | `unix:///var/run/docker.sock`, `npipe://./pipe/docker_engine`, `https://host.example/path?token=x` | unchanged | no `@` | - | `https://user:secret@portainer.example:9443/api/auth?x=1` | `https://***@portainer.example:9443/api/auth?x=1` | parsed with user info | - | `https://user@host.example` | `https://***@host.example/` | parsed with user info | - | `https://:secret@host.example/` | `https://***@host.example/` | parsed with user info | - | `tcp://user:secret@docker.example:2376` | `tcp://***@docker.example:2376/` | parsed with user info | - | `https://user:secret@[::1]:2376/` | `https://***@[::1]:2376/` | parsed with user info | - | `https://user%40x:se%3Acret@host.example` | `https://***@host.example/` | parsed with user info | - | `https://user:secret@host.example/p#frag` | `https://***@host.example/p#frag` | parsed with user info | - | `https://user:p@ss@host.example/path` | `https://***@host.example/path` | fallback | - | `https://user:pa/ss@host.example` | `https://***@host.example` | fallback | - | `npipe://user:secret@./pipe/docker_engine` | `npipe://***@./pipe/docker_engine` | fallback | - | `user:secret@host:2376` | `***@host:2376` | fallback (parsed as scheme `user`, no authority) | - | `user:secret@host?x=http://a` | `***@host?x=http://a` | fallback (`://` only after the `@`) | - | `https://admin:#Xy7secret@portainer:9443` | `https://***@portainer:9443` | fallback (`@`, no parsed user info) | - | `https://admin:?Xy7secret@portainer:9443` | `https://***@portainer:9443` | fallback (`@`, no parsed user info) | - | `https://admin:/Xy7secret@portainer:9443` | `https://***@portainer:9443` | fallback (`@`, no parsed user info) | - | `https://admin:12/34secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | - | `https://admin:12#secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | - | `https://ad/min:secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | - | `https://admin#:secret@host` | `https://***@host` | fallback (`@`, no parsed user info) | - | `tcp://admin:#secret@docker:2376` | `tcp://***@docker:2376` | fallback (`@`, no parsed user info) | - | `https://host.example/@scope/pkg` | `https://***@scope/pkg` | fallback (`@`, no parsed user info; over-masked, accepted in 0005) | - | `https://a:b@c:/secret@host` | `https://***@host` | fallback (more than one `@`) | - | `https:\\user:secret@host.example` | `***@host.example` | fallback (no `scheme://` prefix) | - | `https://***@docker.example.test/` | `https://***@docker.example.test/` | parsed with user info `***` (idempotent) | - -- [ ] AC2: `UrlRedaction.RedactUserInfo(Uri?)` returns `""` for `null`, the AC1 result of `AbsoluteUri` for an - absolute URI (`new Uri("https://user:secret@host.example/a")` → `https://***@host.example/a`) and of - `OriginalString` for a relative one. -- [ ] AC3: A request to `https://user:secret@registry.example.test/v2/path?token=x` that the retry handler retries - logs event `3500` (transient status) and event `3501` (transient exception) with - `https://***@registry.example.test/v2/path` and without `user`, `secret` or `token=x`; the request the inner - handler receives still carries `user:secret` (connecting is unchanged). -- [ ] AC4: With Portainer `BaseUrl` `https://admin:S3cret@portainer.example.test:9443`, user name/password login and a - login answer `401`, `GetCapabilityAsync` returns a `Message` and logs events `3303` and `3306` (message and - exception message) that contain `https://***@portainer.example.test:9443/` and not `S3cret`; the same holds for a - `200` login answer without JWT. -- [ ] AC5: With `BaseUrl` `ssh://user:secret@docker.example.test` and `Enabled = true`, each of - `DiscoverContainersAsync`, `CollectContainerResourceUsageAsync`, `InspectImageAsync`, `GetImageHistoryAsync` and - `GetHostMemoryTotalAsync` returns `Unsupported` with a message containing `ssh://***@docker.example.test/` and no - `secret`; discovery's event `3101` likewise. -- [ ] AC6: With `BaseUrl` `npipe://user:secret@pipehost/`, `DiscoverContainersAsync` returns `Failed` with a message - containing `npipe://***@pipehost/` and no `secret`, and event `3104`'s exception message contains no `secret`. -- [ ] AC7: Synchronization stores `EndpointUri` `https://***@docker.example.test:2376/` for a configured - `https://user:secret@docker.example.test:2376` (connection kind still `Https`, the options' `BaseUrl` unchanged), - stores the Portainer `BaseUrl` masked, logs event `2091` for an enabled `ssh://user:secret@docker.example.test` - masked, and rewrites a stored Portainer `BaseUrl` with user info masked when Portainer is disabled (a stored value already - masked, `https://***@portainer.example.test:9443/`, stays exactly as it is). -- [ ] AC8: `GetDockerInstancesAsync` and `GetDockerInstanceDetailAsync` return `EndpointUri` - `https://***@docker.example.test/` for a stored row whose `EndpointUri` is `https://user:secret@docker.example.test` - (a row written before the change), the already masked `https://***@docker.example.test/` unchanged, and - `unix:///var/run/docker.sock` unchanged. - -## Tests per criterion - -| AC | Test(s) | Test file | -| -- | ------- | --------- | -| AC1 | `UrlRedactionRedactUserInfoNullOrEmptyReturnsEmpty`, `UrlRedactionRedactUserInfoWithoutAtSignReturnsInputUnchanged` (data rows: unix, npipe, query), `UrlRedactionRedactUserInfoAtSignWithoutParsedUserInfoMasked` (data rows: the eight `@`-without-parsed-user-info rows and `@scope/pkg`), `UrlRedactionRedactUserInfoAlreadyMaskedReturnsInputUnchanged`, `UrlRedactionRedactUserInfoUserAndPasswordMasked` (data rows: the seven "parsed with user info" rows), `UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked` (data rows: the five fallback rows of round 1, the `a:b@c:/secret@host` row and the backslash row); every masked row also asserts that neither the user name nor any part of the password of its input (`secret`, `Xy7secret`, `34secret`, `min`, `ss`, `cret`) occurs in the result | `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs` | -| AC2 | `UrlRedactionRedactUserInfoNullUriReturnsEmpty`, `UrlRedactionRedactUserInfoAbsoluteUriMasked`, `UrlRedactionRedactUserInfoRelativeUriUsesOriginalString` | `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs` | -| AC3 | `TransientHttpRetryHandlerTransientStatusLogOmitsUserInfoAndQueryAsync`, `TransientHttpRetryHandlerTransientExceptionLogOmitsUserInfoAndQueryAsync` | `src/Tests/DockerUpdateGuard.Tests/TransientHttpRetryHandlerTests.cs` | -| AC4 | `PortainerClientGetCapabilityAsyncWithUserInfoAndFailedLoginMasksUserInfoAsync`, `PortainerClientGetCapabilityAsyncWithUserInfoAndMissingJwtMasksUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/PortainerClientTests.cs` | -| AC5 | `DockerInstanceClientDiscoverContainersAsyncWithUnsupportedEndpointUserInfoMasksUserInfoAsync`, `DockerInstanceClientOperationsWithUnsupportedEndpointUserInfoMaskUserInfoAsync` (the four other operations) | `src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs` | -| AC6 | `DockerInstanceClientDiscoverContainersAsyncWithNamedPipeWithoutPipeNameMasksUserInfoAsync` | `src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs` | -| AC7 | `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresEndpointWithUserInfoMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncStoresPortainerBaseUrlMaskedAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncUnsupportedEndpointLogMasksUserInfoAsync`, `InstanceDiscoveryServiceSynchronizeConfiguredInstancesAsyncDisabledPortainerMasksStoredBaseUrlAsync` (data rows: unmasked and already masked stored value) | `src/Tests/DockerUpdateGuard.Tests/InstanceDiscoveryServiceTests.cs` | -| AC8 | `ApplicationViewServiceDockerInstancesMaskStoredEndpointUserInfoAsync`, `ApplicationViewServiceDockerInstanceDetailMasksStoredEndpointUserInfoAsync`, `ApplicationViewServiceDockerInstancesKeepAlreadyMaskedEndpointAsync` | `src/Tests/DockerUpdateGuard.Tests/ApplicationViewServiceTests.cs` | - -`RedactUserInfo(null)` is ambiguous between the two overloads: the tests write `RedactUserInfo((string?)null)` and -`RedactUserInfo((Uri?)null)`. - -## Approach - -1. **Helper** `Infrastructure/UrlRedaction.cs`, `internal static class` (pattern of `PaginationLinkGuard`). String - overload, in this order: - 1. `null` or empty → `""`. - 2. No `@` in the input → the input unchanged. - 3. Rebuild only when **all** hold: the input contains exactly one `@`; `Uri.TryCreate(input, UriKind.Absolute, …)` - succeeds; the trimmed input starts with `{uri.Scheme}://` (ordinal, case-insensitive); `UserInfo` is not empty. - Then return `$"{uri.Scheme}://{Mask}@{uri.GetComponents(UriComponents.Host | UriComponents.Port | UriComponents.PathAndQuery | UriComponents.Fragment, UriFormat.UriEscaped)}"`. - There is no "parsed, no user info → unchanged" branch: the parser cannot tell `https://host/@scope` from - `https://admin:/secret@host` (fact 12 b). - 4. Every other input that contains `@` (unparseable, parsed without user info, without an authority, or with more - than one `@`): `last` = last index of `@`; `start` = index after the first `://` when - that occurs before `last`, else `0`; return `input[..start] + Mask + input[last..]`. Fail-closed: everything - before the last `@` after the scheme separator is masked. String operations only, no regular expression. - Uri overload: `null` → `""`; absolute → string overload of `AbsoluteUri`; relative → of `OriginalString`. -2. **Retry handler** `GetRequestTarget`: absolute → `UrlRedaction.RedactUserInfo(requestUri.GetLeftPart(UriPartial.Path))` - (query still dropped first); relative → `OriginalString` cut before its first `?` or `#`, then - `UrlRedaction.RedactUserInfo(…)`, so both branches drop the query as the XML summary says (N1). XML summary - names both. The request itself is untouched. -3. **Output sites**: pass `UrlRedaction.RedactUserInfo(…)` instead of the raw value at `DockerInstanceClient.cs:474` - (Uri overload), `:983`, `:985`, `:1063`, `:1152`, `:1189`, `:1223`; `InstanceDiscoveryService.cs:118`; - `PortainerClient.cs:166`, `:168`, `:173`, `:175` (compute the masked value once per call). Message texts, event - IDs, levels and parameter names (`{BaseUrl}`) stay as they are. -4. **Stored values**: `InstanceDiscoveryService.cs:109` stores `UrlRedaction.RedactUserInfo(configuredInstance.BaseUrl.Trim())`; - `:155` stores `UrlRedaction.RedactUserInfo(configuredInstance.Portainer.BaseUrl?.Trim())`; in the disabled branch - (`:133-137`) also `PortainerEndpoint.BaseUrl = UrlRedaction.RedactUserInfo(PortainerEndpoint.BaseUrl)`. - `MapConnectionKind` keeps reading the options. No migration. -5. **Display**: `ApplicationViewService.cs:2101`, `:2148` map `UrlRedaction.RedactUserInfo(entity.EndpointUri)`. The - pages render the view data as before (no `.razor` change), so the prerendered persistent state is masked too. -6. Layout of output: unchanged except the URL text; no new user-visible element. - -## Options considered - -Weighed in [0005](../../docs/decisions/0005-urls-are-output-with-user-info-masked.md): masking with `***` (chosen), -dropping user info, rejecting it in validation (product decision, breaks working configurations), a log processor -(misses console, messages, database and UI), the issue's anchored regular expression (leaks the part after a second -`@`, misses a `/` in the password), display-only masking (secret stays at rest). One further choice: masking inside -the `LoggerMessage` methods instead of at the call sites — rejected, the exception and result messages need the -masked value too, and one rule per call site is easier to review. - -## Affected projects and types - -| Project | Type / file | Change | -| ------- | ----------- | ------ | -| DockerUpdateGuard | `Infrastructure/UrlRedaction.cs` | new helper | -| DockerUpdateGuard | `Infrastructure/TransientHttpRetryHandler.cs` | `GetRequestTarget` masks user info | -| DockerUpdateGuard | `Docker/DockerInstanceClient.cs` | seven output sites | -| DockerUpdateGuard | `Images/InstanceDiscoveryService.cs` | stored values and event `2091` | -| DockerUpdateGuard | `Portainer/PortainerClient.cs` | event `3303` and two exception messages | -| DockerUpdateGuard | `UI/ApplicationViewService.cs` | two mappings | - -## Signatures (for the Dev's skeleton) - -```csharp -namespace DockerUpdateGuard.Infrastructure; - -internal static class UrlRedaction -{ - public const string Mask = "***"; - public static string RedactUserInfo(string? url); - public static string RedactUserInfo(Uri? uri); -} -``` - -No other signature changes; every other file listed above changes only method bodies. - -## Test files - -- New: `src/Tests/DockerUpdateGuard.Tests/UrlRedactionTests.cs`. -- Extended: `TransientHttpRetryHandlerTests.cs`, `PortainerClientTests.cs`, `DockerInstanceClientTests.cs`, - `InstanceDiscoveryServiceTests.cs`, `ApplicationViewServiceTests.cs` (all in `src/Tests/DockerUpdateGuard.Tests/`). - The retry tests need the log entries: `SendThroughRetryHandlerAsync` gains an optional `TestLogger` parameter (the - Tester's change, test code only). -- Existing test call sites of a changed signature: none. `DockerInstanceClientDiscoverContainersAsyncWithUnsupportedEndpointReturnsUnsupportedAndLogsWarningAsync` - asserts `ssh://docker.example.test` in the log; that value has no `@` and stays unchanged. -- Files no unit test can cover: none. - -## Areas - -- Telemetry: *Credentials and external data* — configured URLs and retry lines carry no user info (masked `***`); - HTTP client spans and client-factory log lines carry none by the platform; known gap #295 removed; 0005 linked. -- Web UI: *Secrets* — endpoints shown with user info masked, also for rows stored before; known gap #295 removed. -- Docker instances: stored endpoint URI masked; messages naming an endpoint mask its user info. -- Container actions: stored base URL masked (also when Portainer is disabled); error messages and logs name the base - URL with user info masked. -- Registries: retry log line carries the URL without user info and query. - -## Documentation updates - -All edits by the **Dev**, after the code, checked against it: - -- `docs/areas/telemetry.md` — *Credentials and external data* bullet 3 ("configured base URLs" → with user info - replaced by `***`; "Retry log lines carry the request URL without its user info and query string"); a bullet that - 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`); - remove the #295 *Known gaps* entry; *Related decisions* → 0005. -- `docs/areas/web-ui.md` — *Secrets*: replace "Docker instance endpoints are shown as configured (see *Known gaps*)" - with the masked rule and an example (`https://***@docker.example:2376/`); remove the #295 *Known gaps* entry; - *Related decisions* → 0005. -- `docs/areas/docker-instances.md` — line 26 "(trimmed `BaseUrl`, its user info masked)"; *Limits and errors*: a - message or log entry that names an endpoint shows it with user info masked. -- `docs/areas/container-actions.md` — line 20 masked `BaseUrl`; line 21 disabled endpoint: base URL rewritten masked; - lines 42 and 88: the base URL is named with user info masked. -- `docs/areas/registries.md` — line 166 "the URL without its user info and query". -- `docs/ARCHITECTURE.md` — *Security posture*, *Secrets in configuration* bullet: one sentence that URLs with user info - are logged, stored and shown with it masked, linking [Telemetry](areas/telemetry.md) and record 0005. -- Sweep done: `grep -i "base url|verbatim|as configured|query string|user info"` over `docs/areas`, `ARCHITECTURE.md`, - `README.md`; the hits above are all that the change makes untrue. `README.md` needs no edit. - -## Architecture check - -No guarantee of `.squad/project.md` is weakened. Clients keep returning `ExternalOperationResult` (only message -text changes); layering stays (helper in the host project, next to `PaginationLinkGuard`); no entity or migration -change. `ARCHITECTURE.md` gets the sentence above because the secrets posture gains a rule. - -## Security considerations - -- [x] No limits on parsed input. Masking can lengthen a value by at most 2 characters (user info shorter than `***`) - plus a trailing `/`; `EndpointUri`/`BaseUrl` columns are 512 characters and configuration has no length check - today, so a configured URL of 509–512 characters with one- or two-character user info would now fail the save that - a 513-character URL fails today. Accepted, not worth a new limit. -- [x] Exceptions reaching a user-visible message: the two Portainer `InvalidOperationException`s (text as today with - the masked URL, e.g. `Portainer authentication failed with HTTP 401 for 'https://***@portainer.example.test:9443/'`) - and the named-pipe `InvalidOperationException` (`The Docker named pipe URI 'npipe://***@pipehost/' does not contain - a pipe name`); runtime `HttpRequestException`s carry no URI (fact 10). -- The helper is the guard: the forms the parser accepts or rejects are enumerated in fact 12, and AC1 has a row for - each class (parsed with user info, `/`, `?` or `#` in the password or user name, numeric port prefix, more than one - `@`, backslash form, no authority, unparseable, already masked). Only a value with exactly one `@` that the parser - resolves to non-empty user info with a `scheme://` authority is rebuilt; every other value with `@` is masked - fail-closed up to its last `@`, at the price of over-masking an `@` in a path (`https://***@scope/pkg`). -- Scan-run error messages stored before the change are not rewritten (fact 3: the unsupported branches are not - reachable in production; the named-pipe message is, and old rows age out with scan-run retention). - -## Decision records - -- `docs/decisions/0005-urls-are-output-with-user-info-masked.md` (Proposed), indexed. - -## Out of scope / follow-ups - -- Trivy's error excerpt (`TrivyVulnerabilityProvider.cs:305-311`) is passed on as Trivy writes it; whether Trivy echoes - a `TrivyBaseUrl` with user info is unknown. Follow-up issue for the orchestrator: title "Trivy error output may echo - credentials embedded in TrivyBaseUrl", body: "`TrivyVulnerabilityProvider` passes the last line of Trivy's stderr - into logs (`TrivyCommandFailed`) and scan-run messages. If `Vulnerabilities:TrivyBaseUrl` carries user info - (`https://user:secret@trivy:4954`), Trivy may repeat it there. Measure Trivy's output for a failing server call with - such a URL, then decide whether to mask user info in free text with the helper of #295 (`UrlRedaction`), or to - reject user info in `TrivyBaseUrl`." -- `OciRegistryClient.cs:1119,1134` name the token realm URI a registry sends; it is not configured and carries no - configured secret — unchanged. -- Rejecting user info in configuration validation: a product decision (0005, option 3), not proposed here. - -## Security review - -- **B1 (blocking) — accepted.** The branch "parsed with authority, empty `UserInfo` → unchanged" is removed (Approach - 1.3). Rebuilding now also requires exactly one `@` (my addition: `https://a:b@c:/secret@host` parses with `UserInfo` - `a:b` and would otherwise keep `secret@host` in the path); everything else with `@` goes to the fallback. AC1 gains - rows for every probe form Security named (including `tcp://admin:#secret@docker:2376`), the `@scope/pkg` row now - expects `https://***@scope/pkg`, and the test is renamed to `UrlRedactionRedactUserInfoAtSignWithoutParsedUserInfoMasked`. - Fact 12, record 0005 (*Decision*, *Consequences*) and *Security considerations* say the same rule. Re-checked the - whole list of accepted forms with the rule itself (fact 12 g): no row leaks a user or password part. -- **N1 — taken.** The relative branch of `GetRequestTarget` cuts `OriginalString` before its first `?` or `#` before - redacting (Approach 2); covered by the existing relative-URI behavior plus the AC3 tests for the absolute branch. -- **N2 — taken.** Re-redacting is idempotent (`***` parses as user info and is rebuilt identically, fact 12 g): AC1 - row, AC7 data row and AC8 test with an already masked stored value. - -## Challenge - -- **Backslash forms (minor) — accepted.** Fact 12 now says `https:\\user:secret@host.example` parses with - `UserInfo` set and, since it does not start with `https://`, goes to the fallback with `***@host.example` (scheme - lost, nothing leaks); AC1 row added to `UrlRedactionRedactUserInfoUnparsableWithUserInfoMasked`. The `scheme://` - prefix condition stays unchanged, keeping `user:secret@host:2376` on the fallback path. - -## Security notes - -Security's non-blocking finding of the plan review (round 2), verbatim: - -**N3.** The fallback (Approach 1.4) starts masking after the first `://` without checking that the text before it is a valid scheme. So `user:secret://x@host` → `user:secret://***@host` and `https:secret://x@host` → `https:secret://***@host`, and both keep `secret`. Neither can reach an output site: -- Validation admits only http, https, tcp, unix and npipe. -- `https:secret://…` does not parse. -- The retry handler only sees parsed http(s) request URIs. - -This does not hold up the plan. Still, using `start` only when the prefix before `://` matches RFC 3986 `ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )` (and otherwise `0`) would make the "fail-closed" claim hold for any input. It needs one AC1 row, `user:secret://x@host` → `***@host`. A one-line change, which can go into this plan.