From ce43f397a6a129c0ac2ba025469587efa5011608 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Tue, 28 Jul 2026 12:28:44 +0200 Subject: [PATCH] Add ADRs 0001-0006 and README (M0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the decisions the milestone plan already made, with their costs stated rather than only their benefits: - 0001 e2ee-trust-model: key hierarchy, the AAD-to-row binding that stops the server moving ciphertext between rows, and the four-layer public-key trust story. States plainly that revocation is not retroactive, that Connect cannot be a security boundary, and that the IdP becomes a key-distribution trust root. - 0002 minimal-apis: feature modules with explicit registration; capability negotiation instead of Asp.Versioning, since client and server upgrade independently when self-hosted. - 0003 sync-protocol: single write path, revision cursors, and the bigserial pre-commit sequence gap that silently corrupts sync — plus the per-vault advisory lock that fixes it and the test that must prove it. - 0004 relay-authorization: relay forwards bytes rather than terminating SSH, so zero-knowledge survives; server-resolved target IPs in the ticket to defeat DNS rebinding; why host addresses must be plaintext when relay is enabled. - 0005 no-application-layer: why the usual Application/mediator layer earns nothing here, with the trigger that would make us revisit it. - 0006 observability-stack: OTel plus built-in ILogger; liveness excludes dependencies so a database blip cannot restart the container and kill live SSH sessions. Also adds a README covering layout, build, enforced conventions and milestones. --- README.md | 96 ++++++++++++++++++++++++ docs/adr/0001-e2ee-trust-model.md | 95 ++++++++++++++++++++++++ docs/adr/0002-minimal-apis.md | 50 +++++++++++++ docs/adr/0003-sync-protocol.md | 83 +++++++++++++++++++++ docs/adr/0004-relay-authorization.md | 101 ++++++++++++++++++++++++++ docs/adr/0005-no-application-layer.md | 55 ++++++++++++++ docs/adr/0006-observability-stack.md | 71 ++++++++++++++++++ 7 files changed, 551 insertions(+) create mode 100644 README.md create mode 100644 docs/adr/0001-e2ee-trust-model.md create mode 100644 docs/adr/0002-minimal-apis.md create mode 100644 docs/adr/0003-sync-protocol.md create mode 100644 docs/adr/0004-relay-authorization.md create mode 100644 docs/adr/0005-no-application-layer.md create mode 100644 docs/adr/0006-observability-stack.md diff --git a/README.md b/README.md new file mode 100644 index 0000000..4a5301c --- /dev/null +++ b/README.md @@ -0,0 +1,96 @@ +# DodoSSH + +A self-hosted, team-oriented SSH client with an end-to-end encrypted vault. + +Manage hosts, credentials and keys in a desktop app; sync them across your devices and share +them with teammates through a server you run yourself. **The server stores ciphertext and never +holds a key** — the operator cannot read the credentials it stores. + +> Status: early development. See [the milestone plan](#milestones) for what exists today. + +## Why + +Teams either scatter SSH credentials across individual `~/.ssh` directories with no sharing +story, or pay per-seat for a hosted product that holds their infrastructure credentials. +DodoSSH keeps the convenience of a synced, shareable vault while remaining self-hostable and +zero-knowledge. + +## Architecture + +| Component | Choice | +| --- | --- | +| Backend | ASP.NET Core on .NET 10, PostgreSQL + EF Core | +| Client | Avalonia (C#) for Windows/Linux/macOS; terminal pane is a WebView running xterm.js | +| Auth | OIDC, provider-agnostic (Entra ID, Keycloak, Auth0, Authentik) | +| Vault | End-to-end encrypted; X25519 + Ed25519 + XChaCha20-Poly1305, Argon2id unlock | +| Connections | Client-direct SSH by default, with an optional raw-TCP server relay | + +Two consequences worth knowing before you read further: + +- **Revocation is not retroactive.** A removed member keeps what they already downloaded. The + real remediation is rotating the SSH credential, so offboarding is built around a rotation + checklist rather than a button that implies more than it delivers. +- **No session recording in relay mode.** The relay forwards SSH ciphertext, so it cannot see + commands. That is the cost of the relay not being able to read your traffic. + +The reasoning behind each major decision is recorded in [`docs/adr/`](docs/adr/), starting with +[the E2EE trust model](docs/adr/0001-e2ee-trust-model.md). + +## Repository layout + +``` +src/ + DodoSSH.Contracts DTOs shared with the client — the real API contract + DodoSSH.Crypto DSH1 envelope, AAD derivation, key wrapping + DodoSSH.Domain entities and invariants, no EF + DodoSSH.Infrastructure DbContext, configurations, migrations + DodoSSH.Api the host +tests/ one test project per source project +docs/adr/ architecture decision records +``` + +## Building + +Requires the .NET SDK pinned in [`global.json`](global.json) (10.0.x). + +```bash +dotnet build DodoSSH.slnx +``` + +```bash +dotnet test DodoSSH.slnx +``` + +Run the API locally: + +```bash +dotnet run --project src/DodoSSH.Api +``` + +It listens on `http://localhost:5233`, serving `/healthz/live`, `/healthz/ready` and — in +Development — `/openapi/v1.json`. + +### Conventions the build enforces + +- Warnings are errors. `dotnet format --verify-no-changes` gates CI. +- Package versions are centralised in `Directory.Packages.props`; `packages.lock.json` is + committed and CI restores in locked mode. +- [`BannedSymbols.txt`](BannedSymbols.txt) bans `DateTime.UtcNow` (use `TimeProvider`), + `Guid.NewGuid` (use `CreateVersion7`), sync-over-async, MD5/SHA1 and PBKDF2. +- Public members of `DodoSSH.Contracts` must be declared in `PublicAPI.Unshipped.txt`, so a + contract change is a build error rather than a client-side surprise. + +## Milestones + +- **M0 — foundation.** Repo structure, build conventions, CI, ADRs. *Done.* +- **M1 — vertical slice.** OIDC login → enroll → create a host → open a shell. Gated on + freezing `DodoSSH.Contracts` and the crypto AAD, plus two client spikes (Linux WebView, + SSH.NET window-change). +- **M2 — full personal vault**, robust sync, relay. +- **M3 — teams**, sharing, ACLs. +- **M4 — hardening and ops**, packaging, self-hosting guide. +- **M5 — multi-provider OIDC**, key rotation, per-item content keys. + +## Licence + +Not yet chosen. diff --git a/docs/adr/0001-e2ee-trust-model.md b/docs/adr/0001-e2ee-trust-model.md new file mode 100644 index 0000000..f77eff9 --- /dev/null +++ b/docs/adr/0001-e2ee-trust-model.md @@ -0,0 +1,95 @@ +# ADR 0001 — End-to-end encrypted vault and its trust model + +- Status: accepted +- Date: 2026-07-28 + +## Context + +DodoSSH stores SSH credentials — passwords, private keys, key passphrases — on a server so +they can sync across a user's devices and be shared with teammates. Authentication is OIDC +against a provider the operator chooses. + +The product is self-hosted. Its buyers are teams who currently refuse to put infrastructure +credentials into a SaaS vault. "Trust us with your production keys" is exactly the promise +we cannot make. + +## Decision + +**The vault is end-to-end encrypted. The server stores ciphertext and never holds a key.** + +1. **A vault passphrase separate from OIDC.** OIDC authenticates but yields no secret we can + derive a key from, and SSO compromise must not equal vault compromise. The passphrase + goes through Argon2id (m=256 MiB, t=4, p=1) to a master key that never leaves RAM. +2. **A per-user keypair, wrapped many ways.** The master key wraps a ~200-byte + `UserSecretBundle` (X25519 + Ed25519 private keys). The *same* bundle is stored under + several independent wraps: passphrase, one per enrolled device, recovery code, and + optionally escrow. A passphrase change therefore re-wraps 200 bytes and updates one row — + no re-encryption of vault data and no coordination with other members. +3. **Vault keys, then per-item data keys.** A vault key is sealed to each member's X25519 + key; each item has its own data key wrapped under the vault key. Rotating a vault key + re-wraps N × 32-byte data keys and never touches content blobs. +4. **AAD bound to row identity.** Every ciphertext's AAD is recomputed from the row's + plaintext columns rather than stored: + `SHA-256("dsh1\n" + purpose + resourceType + resourceId + keyId + keyGeneration + schemaVersion)`. +5. **Layered public-key trust** — see Consequences. + +## Consequences + +### What this buys + +The AAD binding is the most valuable structural property here, and it is not something ACLs +can provide. A malicious server cannot paste credential A's ciphertext onto host B, cannot +roll a row back to an earlier key generation, and cannot replay a revoked grant: each of +those changes the AAD and fails the authentication tag on the client. + +A stolen database dump, a rogue administrator, a TLS-terminating proxy and the relay +operator all see ciphertext only. OIDC account takeover alone yields nothing readable. + +### What it costs, stated plainly + +- **Revocation is not retroactive and cannot be.** A removed member keeps whatever they + already downloaded, along with the cached keys. Rotating the vault key protects only items + written after the rotation. The only real remediation is rotating the SSH credentials + themselves, so offboarding is built around a credential-rotation checklist rather than a + "revoke access" button that implies more than it delivers. +- **`Connect` cannot be a security boundary.** SSH terminates on the client, so opening a + session requires the credential's plaintext on that machine. "May connect but may not view + the key" is unenforceable in this architecture. The flag exists as a UI hint and must never + be documented as access control. +- **Forgotten passphrase with no recovery code and no enrolled device means permanent loss** + of personal vault contents. Team vault contents survive, because a remaining member with + `Share` can re-wrap. That asymmetry is a feature: it makes team vaults the right default + even for a team of one plus a backup admin. +- **Public-key distribution is the real security boundary.** Every guarantee is downstream of + "the key I wrapped to is really Alice's". Four layers, deployed together: an IdP-signed key + binding (the enrollment statement's hash is the `nonce` in an ID token, verified against + JWKS fetched directly from the IdP and not proxied through us); TOFU fingerprint pinning + with blocking warnings; an append-only key log whose head is embedded in every signed + grant, so a forked view must stay consistent forever to go unnoticed; and safety numbers + for out-of-band verification. + + The residual is honest and must stay in the docs: this makes the IdP a key-distribution + trust root, and in a self-hosted deployment the person running Keycloak is frequently the + person running DodoSSH. It raises the bar from "one compromised service" to "one + compromised service plus a detectable artefact in the key log" — not to zero. +- **No server-side session recording is possible** in relay mode, since the relay forwards + only SSH ciphertext. See [ADR 0004](0004-relay-authorization.md). +- **Metadata leaks.** The server sees item counts, sizes, timestamps, access patterns and the + complete sharing graph regardless of settings. Host addresses are plaintext when relay is + enabled for that host; see [ADR 0004](0004-relay-authorization.md) for why that is a + security requirement rather than a convenience. +- **Supply chain becomes the largest practical hole.** An operator who wants the secrets + attacks the client, not the crypto. Release signing with a key not held by the server, and + eventually reproducible builds, matter more here than in a conventional product. + +### Rejected + +- **Server-side envelope encryption** (KMS-held master key). Far simpler and it would permit + a recording bastion, but a server compromise or a rogue admin exposes every credential. + That is the exact promise the product exists to avoid making. +- **Admin escrow of user identity keys.** Turns every operator into a silent global reader + and destroys the property being sold. Non-negotiable. Team-scoped break-glass escrow with + Shamir M-of-N is a separate, opt-in, clearly-labelled M5 feature. +- **Storing an `Argon2id(passphrase)` verifier server-side** so the server can pre-validate. + It creates an offline-crackable verifier on the very server being defended against, for no + gain: the AEAD tag on the bundle wrap already proves the passphrase. diff --git a/docs/adr/0002-minimal-apis.md b/docs/adr/0002-minimal-apis.md new file mode 100644 index 0000000..4408389 --- /dev/null +++ b/docs/adr/0002-minimal-apis.md @@ -0,0 +1,50 @@ +# ADR 0002 — Minimal APIs with explicitly registered feature modules + +- Status: accepted +- Date: 2026-07-28 + +## Context + +The API is roughly 60 endpoints: identity and enrollment, a public-key directory, teams, +vaults and grants, sync, read-only item queries, relay, audit and admin. It is consumed by +one first-party desktop client. + +## Decision + +Minimal APIs, grouped into feature modules under `Features/`, registered **explicitly** from +`Setup/EndpointRegistration.cs`. Typed results (`Results, ForbidHttpResult, +ProblemHttpResult>`) throughout, one endpoint per file. + +Version with a hard-coded `/api/v1` prefix and **no `Asp.Versioning` package**. Instead ship +capability negotiation: + +- `GET /api/v1/meta` — server version, sync protocol version, crypto spec version, feature + flags, `minClientVersion`, and the push caps. +- `GET /.well-known/dodossh-configuration` — OIDC authority, client id, scopes, relay URL. + +## Consequences + +- Per-endpoint filters and metadata compose cleanly, and typed results give an accurate + OpenAPI document without attribute noise. +- Explicit registration over reflection scanning: predictable startup cost, trimming-friendly, + and every route is greppable. The cost is one line per module, which is worth paying. +- Minimal APIs' real weakness is that a cross-cutting concern is easy to forget. Two + mitigations, both required: group-level `RequireAuthorization`, and an **endpoint-inventory + test** that enumerates `EndpointDataSource` and fails the build if any endpoint is absent + from an explicit authorization expectations table. A new endpoint therefore cannot be added + without making an authorization decision. +- Capability negotiation over version routing is the right trade for a self-hosted product, + where client and server upgrade independently and skew is normal rather than exceptional. + `.well-known` also *is* the onboarding story: the user types one server URL and the client + discovers the rest. `Asp.Versioning` gets added when a v2 actually exists, not before. +- The generated OpenAPI document is for third parties and a future CLI. It is emitted at build + time to `artifacts/openapi/v1.json`, committed and CI-diffed. **The client's real contract is + the `DodoSSH.Contracts` assembly**, guarded by PublicApiAnalyzers so an accidental DTO change + is a build error rather than a runtime deserialisation failure on a user's laptop. + +### Rejected + +- **MVC controllers.** Would work, but bring filter/model-binding machinery this API does not + need and blur the one-endpoint-one-file structure that keeps the authz surface reviewable. +- **Reflection-based endpoint discovery.** Convenient until a route silently disappears + because an assembly was not loaded, or trimming removes it. diff --git a/docs/adr/0003-sync-protocol.md b/docs/adr/0003-sync-protocol.md new file mode 100644 index 0000000..cdfbda5 --- /dev/null +++ b/docs/adr/0003-sync-protocol.md @@ -0,0 +1,83 @@ +# ADR 0003 — Revision-based delta sync through a single write path + +- Status: accepted +- Date: 2026-07-28 + +## Context + +Clients must work fully offline and reconcile on reconnect, across multiple devices per user. +The server holds ciphertext, so **it cannot merge, validate or inspect item contents**. Every +conflict resolution decision therefore has to happen on a client. + +## Decision + +### One write path + +All vault mutations go through `POST /vaults/{vaultId}/sync/push`. There are no per-entity +POST/PUT/DELETE endpoints. Reads are separate, list/get only, keyset-paginated. + +### Change log and cursors + +A per-vault `sync_change(seq bigserial, vault_id, entity_type, entity_id, operation, +revision, actor_user_id, occurred_at_utc)` log. Each entity denormalises `change_seq` so a +delta pull joins straight to the row. + +Cursors are opaque and HMAC-tagged — `base64url("v1|{vaultId}|{seq}")` — so a tampered cursor +is rejected rather than silently mis-serving someone else's data. + +### Push semantics + +`expectedVersion` per operation. **HTTP 200 even on partial failure**, with a per-operation +status of `Applied | Conflict | Forbidden | Invalid | Duplicate`. Conflicting operations are +skipped, not aborted, and a `Conflict` returns the server's current row so the client can +merge and re-push. `opId` deduplication via `sync_operation_receipt` makes a retried push +exactly-once at operation granularity. + +Caps enforced before the transaction opens: 500 operations per push, 8 MiB per batch, +256 KiB per item. + +### The `bigserial` cursor gap — the reason for the advisory lock + +`bigserial` hands out values **before** commit. Transaction A takes seq 5, B takes 6 and +commits first; a reader that advances its cursor to 6 **permanently misses 5**. This is silent +sync corruption that only manifests under concurrent writes to a single vault, which is +exactly the case least likely to be exercised by hand. + +Mitigation: every push takes, as its first statement, + +```sql +SELECT pg_advisory_xact_lock(hashtextextended(@vaultId::text, 0)) +``` + +This serialises writers per vault, so sequence order equals commit order. Contention is +per-vault and a push batch is already one transaction. **It requires `Multiplexing=false` in +the Npgsql connection string** — the default; do not enable multiplexing. + +## Consequences + +- One place enforces revision, change-log and ACL invariants. That halves both the endpoint + count and the authorization surface, which is the main reason for the single write path. +- Delta pull makes frequent polling cheap, so multi-device feels live; push notification over + SSE or the existing WebSocket can layer on with polling as the fallback. +- Conflict resolution is entirely client-side. The client retains a `BaseCiphertext` common + ancestor and performs a field-level three-way merge for structured items, or creates a + visible conflicted copy for opaque ones. **It must never silently drop a key or a host.** +- Deletes are revisioned tombstones, garbage-collected after 90 days. Sync must therefore be + able to read tombstones, which is the one place that legitimately bypasses the soft-delete + query filter — guarded by an explicit permission check. +- **An `Infrastructure.Tests` case must prove cursor ordering under N concurrent pushes.** + Without it this ADR's central bug is invisible until production. +- Two concurrency mechanisms, deliberately: `version integer` is the client-visible monotonic + item version used for conflict detection; `xmin` is the server-side optimistic guard and is + **never exposed**, because it is not stable across `VACUUM FREEZE` and must not become a + client cursor. + +### Rejected + +- **Snapshot-watermark cursors** (`pg_snapshot_xmin(pg_current_snapshot())`). Correct without + locking, but materially harder to reason about and to test. Revisit only if per-vault lock + contention shows up in practice. +- **Last-writer-wins.** Cheap, and it loses credentials. Unacceptable for this data. +- **Full pull on every sync.** Simple, but rules out the frequent polling that makes + multi-device sync feel immediate. +- **Server-side merge.** Impossible by construction: the server cannot read the payloads. diff --git a/docs/adr/0004-relay-authorization.md b/docs/adr/0004-relay-authorization.md new file mode 100644 index 0000000..653b131 --- /dev/null +++ b/docs/adr/0004-relay-authorization.md @@ -0,0 +1,101 @@ +# ADR 0004 — Relay as a raw TCP tunnel, and how its targets are authorized + +- Status: accepted +- Date: 2026-07-28 + +## Context + +Some hosts are not reachable from a user's laptop. The obvious answer is a server-side jump +host — but that conflicts directly with [ADR 0001](0001-e2ee-trust-model.md): a server that +terminates SSH needs plaintext credentials for the target. + +A server that dials arbitrary addresses on an authenticated user's behalf is also a textbook +SSRF and lateral-movement primitive, aimed at the operator's own network. + +## Decision + +### The relay forwards bytes, it does not terminate SSH + +The client opens a WebSocket to the backend; the backend pipes raw bytes to `host:port`. The +SSH handshake still terminates on the client, so the relay sees only SSH ciphertext. +Zero-knowledge survives. Subprotocol `dodossh.relay.v1`; no framing of our own, because SSH is +already a byte stream. + +### Two steps, so the WebSocket carries no API authority + +1. `POST /relay/tickets` — full bearer JWT, full ACL context. Takes **`{ hostId }`, never a + client-supplied address.** +2. `GET /relay/connect` — WebSocket upgrade with the ticket only, zero API access. + +Tickets are `IDataProtectionProvider` payloads (key management and rotation for free) carrying +`jti, userId, deviceId, hostId, targetIps[], targetPort, exp ≤ 30s`, single-use via a unique +insert into `relay_ticket_use`. The ticket travels in +`Sec-WebSocket-Protocol: dodossh.relay.v1, ticket.`, since constrained WebSocket clients +cannot set arbitrary headers; a `?ticket=` fallback exists and **requires query scrubbing in +access logs and OTel spans**. + +### Anti-SSRF, in order of importance + +1. **The server resolves the target from `host_id`.** `relay_enabled = true` and non-null + `hostname`/`port` are enforced by a database CHECK constraint. Arbitrary targets are not + supported in v1 at all. +2. **DNS resolves at ticket-issue time and the resolved IPs go into the ticket; the relay + dials the IP, never the name.** This is what defeats DNS rebinding, which otherwise breaks + naive allow-listing. +3. **A non-overridable deny list with no configuration escape:** loopback, link-local, + `0.0.0.0/8`, multicast, cloud metadata (`169.254.169.254`, `100.100.100.200`, + `fd00:ec2::254`), and **IPv4-mapped IPv6 normalised and then re-checked** — a classic + bypass. +4. **A configurable layer:** `Relay:AllowPrivateNetworks` defaults **true**, because RFC1918 is + the primary use case for a self-hosted SSH tool, plus allow/deny CIDRs. Ports allow + everything except SMTP. Database ports stay open, because forwarding to 5432 is a headline + feature — **the real control is the ACL**: you can only dial hosts in a vault you hold + `Connect` on. +5. **Limits:** 10 concurrent sessions per user, 200 per node, 12 h maximum, 10 min idle, 5 s + connect. + +### Why host addresses are plaintext + +Encrypting hostnames sounds strictly better and is not. The relay must resolve its target +server-side or point 1 above collapses and the relay becomes an authenticated open TCP proxy — +a worse risk than the metadata it would protect. Plaintext address and port are therefore +stored **only when the user opts that host into relay**, enforced by the CHECK constraint. +Everything else about a host — username, notes, jump chain, options — is always ciphertext, and +there is no plaintext host label at all, because ACL administration runs on the client, which +can decrypt names. + +Searchability is explicitly *not* the argument: vaults hold thousands of items, not millions, +so the client syncs everything and searches in memory. The relay and audit arguments are the +real ones. + +## Consequences + +- **No session recording or command auditing is possible in relay mode.** The relay sees + ciphertext. Do not promise otherwise. Recording would need a separate, explicitly + non-zero-knowledge "recorded bastion" mode opted into per host. +- Useful audit remains: `target_host:port`, duration, byte counts, close reason, client IP. +- Backpressure comes from `System.IO.Pipelines` with `pauseWriterThreshold: 1 MiB`. When the + WebSocket peer is slow, `FlushAsync` stops completing, which stops reading the socket, which + lets TCP's own receive window throttle the origin end-to-end. No custom flow control, ~2 MiB + bounded per session — which is what makes 200 sessions per node viable. +- **The relay must not hold a `DbContext` for the session lifetime.** Insert the row, dispose + the scope, relay, then open a fresh scope to write the close row. Otherwise the connection + pool dies around 100 concurrent sessions. +- Tickets are stateless-verifiable and `relay_ticket_use` is the only shared state, so any node + accepts any ticket: plain round-robin, no session affinity. That table is also the extraction + seam — a standalone relay process needs the Data Protection key ring and one table, no ACL + code. Extraction is worthwhile eventually because long-lived connections and short requests + have opposite scaling and rollout profiles. +- Graceful shutdown sends close 1001 and drains for 30 s, so a `docker compose up -d` does not + guillotine live shells. +- On the client, SSH.NET cannot be handed a pre-connected stream, so the relay is reached via a + loopback TCP bridge. The same bridge provides ProxyJump via a SOCKS5 dynamic forward — one + mechanism, two features. + +### Rejected + +- **Server terminates SSH (a true bastion).** Enables recording and central credential + control; destroys zero-knowledge. Out of scope, possibly a separate product line. +- **Client-supplied target address.** One line of convenience, and the relay becomes an open + proxy into the operator's network. +- **Allow-listing by hostname.** Defeated by DNS rebinding. diff --git a/docs/adr/0005-no-application-layer.md b/docs/adr/0005-no-application-layer.md new file mode 100644 index 0000000..eb14055 --- /dev/null +++ b/docs/adr/0005-no-application-layer.md @@ -0,0 +1,55 @@ +# ADR 0005 — No separate Application layer + +- Status: accepted +- Date: 2026-07-28 + +## Context + +A conventional layering for a .NET service of this size would be Domain → Application → +Infrastructure → Api, with a mediator and one handler per use case. + +DodoSSH's server is, in substance, CRUD plus one interesting write path +([ADR 0003](0003-sync-protocol.md)) plus a byte relay ([ADR 0004](0004-relay-authorization.md)). +It performs **no cryptography on secrets and no business logic over item contents**, because it +cannot read them. The genuinely hard logic — merge, key wrapping, trust decisions — lives on +the client. + +## Decision + +Three projects: `DodoSSH.Domain` (entities, enums, invariants, no EF), `DodoSSH.Infrastructure` +(DbContext, configurations, migrations, queries), `DodoSSH.Api` (host, with vertical slices +under `Features/`). Plus `DodoSSH.Contracts` for shared DTOs and `DodoSSH.Crypto` for the +envelope format. + +No `Application` project, no mediator. + +## Consequences + +- A feature is one folder under `Features/`: endpoint, request/response mapping, and the query + or command inline. Reading an endpoint means reading one file, not tracing a request through + a handler, a validator, a behaviour pipeline and a repository interface. +- The testability argument for an Application layer does not apply here. The tests that matter + for this server are authorization tests and sync-protocol tests, and both must run against a + real PostgreSQL to be meaningful — an in-memory handler test would pass while the EF query + filter, the advisory lock and the CHECK constraints all went unexercised. Integration tests + with Testcontainers are the primary suite, so an extra seam buys nothing. +- Business invariants that are genuinely invariant (permission algebra, relay target + validation, cursor encoding) live in `Domain` as pure functions and are unit-tested there. + That is where the fast tests belong. +- **Accepted risk:** if the server later grows real domain logic — server-side policy + evaluation, workflow, notifications with side effects — endpoints will start to get long. + The mitigation is a rule, not a layer: when an endpoint file exceeds ~80 lines, extract a + named service into `Features//`. Revisit this ADR if that happens three times in one + area. +- `DodoSSH.Crypto` is referenced by the API but only for format and fingerprint constants. It + must stay free of any code path that could decrypt a payload server-side, so that the + dependency cannot quietly become a capability. + +### Rejected + +- **Application layer with MediatR.** Adds a hop and a handler per feature. Its usual payoffs — + transaction/validation/logging behaviours, and decoupling from the web framework — are either + already provided (endpoint filters, ProblemDetails) or irrelevant (there is no second host). +- **Repository interfaces over EF Core.** `DbContext` is already a unit of work and EF's global + query filters *are* the authorization backstop ([ADR 0002](0002-minimal-apis.md)). Wrapping + it would hide the mechanism that keeps the API failing closed. diff --git a/docs/adr/0006-observability-stack.md b/docs/adr/0006-observability-stack.md new file mode 100644 index 0000000..4727c75 --- /dev/null +++ b/docs/adr/0006-observability-stack.md @@ -0,0 +1,71 @@ +# ADR 0006 — OpenTelemetry with built-in ILogger, not Serilog + +- Status: accepted +- Date: 2026-07-28 + +## Context + +The server is self-hosted by people who did not write it and who will debug it themselves. It +also handles data it cannot read, which changes what observability can and should capture. + +## Decision + +**Logging:** the built-in `ILogger` with source-generated `[LoggerMessage]` partial methods. +No Serilog. `AddJsonConsole` outside Development; self-hosters capture container stdout. + +**Telemetry:** OpenTelemetry via `OpenTelemetry.Extensions.Hosting` with ASP.NET Core, HttpClient +and runtime instrumentation, Npgsql's own `ActivitySource`, and custom sources `DodoSSH.Api`, +`DodoSSH.Sync`, `DodoSSH.Relay`. OTLP exporter honouring the standard `OTEL_EXPORTER_OTLP_ENDPOINT` +and `OTEL_SERVICE_NAME` variables, because self-hosters expect them. Sampling +`ParentBased(TraceIdRatioBased(0.1))` by default, with tail sampling for errors done in the +collector. Optional Prometheus scrape on a separate port, off by default. + +**Redaction is a hard requirement, not a nicety.** `Microsoft.Extensions.Compliance.Redaction` +with `[PrivateData]` on DTO properties, plus an `ActivityProcessor` that strips `url.query` — the +relay ticket fallback lives there ([ADR 0004](0004-relay-authorization.md)) — and drops +`Authorization` and `Idempotency-Key`. Sync request bodies are never logged: they are ciphertext, +but size and shape still leak. + +**Health:** `/healthz/live` checks the process only. `/healthz/ready` additionally checks +PostgreSQL, OIDC discovery and JWKS reachability, the Data Protection key ring, and that no +migrations are pending. + +## Consequences + +- One telemetry pipeline instead of two configuration systems. OTel logs are first-class in + .NET 10, so Serilog's usual payoff — sinks and enrichers — is redundant once a collector is in + the compose file, and dual configuration is a real support burden ("why is my log level being + ignored?"). If someone wants file logs, that is the collector's file exporter, not a second + logging framework. +- `[LoggerMessage]` is allocation-free and produces structured events by construction. CA1848 is + a warning during early development and is raised to error when the logging pass lands in M4. +- **Liveness deliberately excludes dependencies.** A transient PostgreSQL outage must not cause + the orchestrator to restart the container, because that would kill every live relay session for + a fault that has nothing to do with them. Readiness only removes the instance from load + balancing, which is the correct response. +- **Audit is a separate concern from logs.** `audit_event` is a partitioned, append-only table + (`REVOKE UPDATE, DELETE` from the application role) with a per-day hash chain. It records who, + when, which item id, which operation, outcome and source — plus non-secret shape data such as + `fieldsChanged` and `payloadBytes`. Its `detail` jsonb must contain ids, counts and *field + names* only, enforced by a serializer whitelist and a test. +- Given no plaintext is visible, audit is nonetheless strong on the thing that matters: every + ciphertext fetch is recorded, and there is no server-side path that reads a secret without a + client fetch. Bulk fetch of an entire vault is a high-signal exfiltration indicator and should + alert. +- Honest limit on tamper evidence: a compromised server can rewrite the chain from any point + forward unless heads are anchored externally. Publishing the daily head to clients, which cache + it, makes truncation detectable — the same trick as the key log in + [ADR 0001](0001-e2ee-trust-model.md). +- Human-readable audit is a **client** concern: rows carry `subject_id`, and the client, which + holds the keys, resolves ids to names. +- Behind a reverse proxy, `UseForwardedHeaders` needs `KnownProxies`/`KnownNetworks` configured. + **Log a startup warning if forwarded headers are enabled with no known proxies**, because + self-hosters will get this wrong and it silently turns per-user rate limiting into per-proxy. + +### Rejected + +- **Serilog.** Excellent library; the second configuration system is the problem, not the code. +- **Application Insights or another vendor SDK.** Wrong for a self-hosted product. OTLP lets the + operator point at whatever they already run. +- **Logging request bodies for sync.** Ciphertext, so it looks harmless, and it leaks item sizes + and access patterns while ballooning log volume.