[Security][low] Bearer confirmation token logged in plaintext (OrganizationsController:118) — only such site #441

Closed
opened 2026-07-06 07:22:08 +00:00 by spikerj · 1 comment
Owner

Finding (low severity, isolated)

SpikerSoft.Api/Domain/Organizations/OrganizationsController.cs:118 logs a raw organization-confirmation token value to Seq on the InvalidOperationException path:

_logger.LogWarning(ex, "Confirmation failed with token: {Token}", token);

Confirmation tokens are bearer secrets (whoever holds a still-valid one can confirm). Logging the value means it persists in Seq (indexed, exportable, screenshot-able). "On failure" only partially mitigates — a transient InvalidOperationException can log a token that is still valid.

Good news — this is the ONLY site

Swept all Log* calls for raw secret/PII values (not just the word "token" in a message): every other auth/token log statement logs identifiers ({UserId}, {PreRegId}, {RequestId}, {StatusCode}) or the literal word "token", never a secret value. Open-redirect sweep also clean (the one Response.Redirect is a same-host HTTP→HTTPS upgrade). So closing this one line makes "no bearer secrets in logs" hold codebase-wide.

Fix (trivial, deferred)

Drop {Token}/token from the template — the exception message + surrounding context are enough to diagnose (optionally log a short non-reversible prefix/hash if a correlation handle is wanted).

Why filed, not PR'd right now

Low severity + a single one-line log-template change that isn't naturally unit-testable, against a currently 7-deep review queue. Batching it into a small log-hygiene cleanup once the queue drains is a better use of review attention than a standalone marginal PR. Happy to ship immediately if you'd prefer.

## Finding (low severity, isolated) `SpikerSoft.Api/Domain/Organizations/OrganizationsController.cs:118` logs a **raw organization-confirmation token** value to Seq on the `InvalidOperationException` path: ```csharp _logger.LogWarning(ex, "Confirmation failed with token: {Token}", token); ``` Confirmation tokens are bearer secrets (whoever holds a still-valid one can confirm). Logging the value means it persists in Seq (indexed, exportable, screenshot-able). "On failure" only partially mitigates — a *transient* `InvalidOperationException` can log a token that is still valid. ## Good news — this is the ONLY site Swept all `Log*` calls for raw secret/PII **values** (not just the word "token" in a message): every other auth/token log statement logs identifiers (`{UserId}`, `{PreRegId}`, `{RequestId}`, `{StatusCode}`) or the literal word "token", **never a secret value**. Open-redirect sweep also clean (the one `Response.Redirect` is a same-host HTTP→HTTPS upgrade). So closing this one line makes "no bearer secrets in logs" hold codebase-wide. ## Fix (trivial, deferred) Drop `{Token}`/`token` from the template — the exception message + surrounding context are enough to diagnose (optionally log a short non-reversible prefix/hash if a correlation handle is wanted). ## Why filed, not PR'd right now Low severity + a single one-line log-template change that isn't naturally unit-testable, against a currently 7-deep review queue. Batching it into a small log-hygiene cleanup once the queue drains is a better use of review attention than a standalone marginal PR. Happy to ship immediately if you'd prefer.
Author
Owner

Resolved in spikersoft-backend PR #131 (merged to master as 67a7ac9). The raw confirmation token is no longer written to the log; regression test asserts the token value never reaches the log state. "No bearer secrets in logs" now holds codebase-wide. Closing.

Resolved in spikersoft-backend PR #131 (merged to `master` as `67a7ac9`). The raw confirmation token is no longer written to the log; regression test asserts the token value never reaches the log state. "No bearer secrets in logs" now holds codebase-wide. Closing.
Sign in to join this conversation.