[Bug][Backend][API] UploadBookCommandHandler.Lean reads its own RabbitMQ:* config with hardcoded 'dockerUser/dockerPassword' defaults — bypasses the shared messaging config path #574

Closed
opened 2026-07-14 14:59:07 +00:00 by spikerj · 1 comment
Owner

QA Team — filed 2026-07-14 from the README audit, at spikerj's direction (decision: file).

The bug: SpikerSoft.Api/.../UploadBookCommandHandler.Lean.cs reads RabbitMQ:HostName / :Port / :Username / :Password directly and falls back to hardcoded defaults dockerUser / dockerPassword, while every other messaging path in the codebase goes through the shared connection/config (Rabbit:* via the common factory, with the singleton IConnection that has automatic recovery, heartbeats and topology recovery configured).

Three consequences:

  1. Silent misconfiguration risk — if the RabbitMQ:* keys are absent in an environment, this handler doesn't fail loudly; it tries dockerUser/dockerPassword and fails at connect time with a credentials error that looks like a broker problem.
  2. Divergent connection semantics — this path doesn't inherit the shared connection's recovery settings.
  3. Credentials in source — even as defaults, baked-in usernames/passwords are exactly what the OpenBao epic (#543) is meant to eliminate.

Note the handler also hardcodes its exchange/routing values (upload.lifecycle / upload.received) inline rather than taking them from config — worth normalizing in the same change.

Fix shape: use the shared RabbitMQ connection/config path like every other publisher; delete the hardcoded credential defaults; move the exchange/routing-key strings to config or a shared constants location.

**QA Team** — filed 2026-07-14 from the README audit, at spikerj's direction (decision: file). **The bug:** `SpikerSoft.Api/.../UploadBookCommandHandler.Lean.cs` reads `RabbitMQ:HostName` / `:Port` / `:Username` / `:Password` directly and falls back to **hardcoded defaults `dockerUser` / `dockerPassword`**, while every other messaging path in the codebase goes through the shared connection/config (`Rabbit:*` via the common factory, with the singleton `IConnection` that has automatic recovery, heartbeats and topology recovery configured). Three consequences: 1. **Silent misconfiguration risk** — if the `RabbitMQ:*` keys are absent in an environment, this handler doesn't fail loudly; it tries `dockerUser/dockerPassword` and fails at connect time with a credentials error that looks like a broker problem. 2. **Divergent connection semantics** — this path doesn't inherit the shared connection's recovery settings. 3. **Credentials in source** — even as defaults, baked-in usernames/passwords are exactly what the OpenBao epic (#543) is meant to eliminate. Note the handler also hardcodes its exchange/routing values (`upload.lifecycle` / `upload.received`) inline rather than taking them from config — worth normalizing in the same change. **Fix shape:** use the shared RabbitMQ connection/config path like every other publisher; delete the hardcoded credential defaults; move the exchange/routing-key strings to config or a shared constants location.
Author
Owner

Resolved in spikersoft-backend PR #286 (merged to master).

The hardcoded credentials are gone, but the more serious bug was one this ticket didn't mention: both publish sites were fire-and-forget — a ConnectionFactory built inline, published without publisher confirms, on the ingest path. A broker hiccup silently dropped the upload and the user's book simply never appeared, with nothing logged.

There were also two copies of that code, not one: UploadBookCommandHandler.Lean.cs and a second identical copy in BookController (the staff-approval path) that the ticket didn't cover.

Both now route through the shared IMessageBusPublisher (publisher confirms + W3C trace context), against one canonical contract in SpikerSoft.Common/Messaging/UploadLifecycleMessaging.cs — replacing the worker's private duplicate UploadReceivedEvent, which was a schema-drift break waiting to happen.

The old BookControllerTests had a catch (BrokerUnreachableException) swallowing the publish failure so the test passed without a broker; that kludge is deleted and the test now Verifys the publish actually happens.

Closing.

Resolved in spikersoft-backend PR #286 (merged to `master`). The hardcoded credentials are gone, but the more serious bug was one this ticket didn't mention: both publish sites were **fire-and-forget** — a `ConnectionFactory` built inline, published without publisher confirms, on the *ingest* path. A broker hiccup silently dropped the upload and the user's book simply never appeared, with nothing logged. There were also **two** copies of that code, not one: `UploadBookCommandHandler.Lean.cs` and a second identical copy in `BookController` (the staff-approval path) that the ticket didn't cover. Both now route through the shared `IMessageBusPublisher` (publisher confirms + W3C trace context), against one canonical contract in `SpikerSoft.Common/Messaging/UploadLifecycleMessaging.cs` — replacing the worker's private duplicate `UploadReceivedEvent`, which was a schema-drift break waiting to happen. The old `BookControllerTests` had a `catch (BrokerUnreachableException)` swallowing the publish failure so the test passed without a broker; that kludge is deleted and the test now `Verify`s the publish actually happens. Closing.
Sign in to join this conversation.