Public Access
Add ADRs 0001-0006 and README (M0)
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.
This commit is contained in:
@@ -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.
|
||||
Reference in New Issue
Block a user