Commit Graph
4 Commits
Author SHA1 Message Date
jaap-jan e24012b039 Sync credentials as a vault item type, and bind one to a host
Closes the largest remaining M1 gap in the data layer: a username and password
can live in the vault, sync between machines, and be named by a host as how it
authenticates. What is not here is the interface for creating one — see the end
of this message.

The third item type, and the first one that cost almost nothing to add. Server:
a VaultCredential row, an EF configuration, a migration, and a CredentialKind.
Client: a secret, a codec, a merge, a cipher, a kind, a repository facade and a
session property. No new reconciliation logic, no change to the sync engine, no
client cache migration. That was the whole point of the item-kind seam, and this
is the evidence it holds.

The narrowest type of the three on plaintext, and not for symmetry. A host has a
deliberate concession — the relay needs an address it can resolve. A key has a
fingerprint, public by nature, which this client still declines to send. A
password has no part that is safe to expose: not its length, not a hash, not a
hint. So CredentialKind refuses every plaintext field there is, hydrates none,
and the table has no column to put one in.

HostSecret.CredentialId is the password counterpart of SshKeyId, and the two are
mutually exclusive. SSH itself would happily try a key and fall back to a
password, but a host naming both leaves "how does this authenticate?" without a
single answer — the interface, the connect path and the user would each be free
to guess differently. TryValidate refuses it. One consequence was not
anticipated: "a full host" stops being a coherent idea, which is what broke
AFullHost_RoundTrips and is now written into that test.

The schema version became a ladder rather than a maximum: credential-bound is 3,
key-bound is 2, neither is still 1. Adding credentials therefore does not drag
every key-bound host in every vault onto a version that clients understanding
keys perfectly well would refuse to edit. A test pins exactly that, because it is
the property the whole content-dependent-version rule exists to provide, and the
obvious implementation would quietly lose it.

Two tests had become false and said so:

- Push_AnUnsupportedEntityType_IsInvalidNotAFailedBatch used Credential as its
  example of a type this server does not implement. It now asks the server's own
  registry what is still missing, so it cannot go stale again, and skips with a
  reason if that set ever empties.
- ThePullFilterNamesEveryTypeThisBuildSynchronises pinned the exact list, which
  is what it is for.

Also fixes ten nullable warnings — eight in SyncEndpointTests, two in a test file
added earlier today. Neither set was introduced here; both were invisible until
an unrelated change forced their project to recompile, which means the
zero-warning claims made earlier in this work only ever covered what happened to
be rebuilt.

777 tests green. Zero warnings, dotnet format clean.

Not done, and deliberately: the credential interface. The vault column is 340
pixels wide and already holds two lists and two editors, kept from clipping its
own buttons at the window's minimum height only by the one-editor-at-a-time rule
added earlier today. A third list and a third editor would recreate that defect
rather than avoid it, so the column needs a shape decision first. Credentials
sync; they cannot yet be created in the interface.
2026-07-29 21:09:08 +02:00
jaap-jan e93acc856f Sync SSH keys as a vault item type, over a shared write path
The private key now lives in the vault as ciphertext, syncs between a user's
machines, and is stored on the server so it can later be shared — sharing
itself needs M3's signed grants; this is the storage that makes it possible.

More was already reserved than expected: SyncEntityType.SshKey,
CryptoSpec.AadResourceType.SshKey, ChangeEntityType.SshKey,
SyncPlaintextFields.PublicKeyFingerprint, and SshPrivateKeyCredential wired
through PrivateKeyFile over a MemoryStream so a key never touches disk. The
frozen contract and crypto spec needed no change at all.

What was missing was the server. Rather than copy the push path per item type
— version check, change-log append, exactly-once receipt, advisory lock — it
is now written once over IVaultItem, with everything type-specific behind
IItemKind: which table, which plaintext columns, and what those columns must
satisfy. Ten copies of that logic by M5, with a fix applied to nine, is the
outcome this avoids. The refactor landed first with no behaviour change, so
all 66 existing Host tests were the regression net, and they stayed green.

An interface rather than a base class, deliberately: EF Core maps an
inheritance hierarchy when it can see one, so a mapped base would quietly
become a table-per-hierarchy discriminator across item types — the very
arrangement per-type tables exist to avoid.

ssh_key mirrors host and pointedly has no relay trio. That is the argument
for separate tables rather than one wide item table: the columns a host needs
are columns a key must never have, and a shared table could only make them
nullable and trust the code. A key carrying a relay target is refused with a
reason rather than silently dropped.

A key hydrates PlaintextFields as null, not an empty instance — the
difference is visible on the wire, because an all-defaults instance still
serialises "relayEnabled": false and invites a reader to believe the setting
exists and is off. It has none.

Two things now defended by tests rather than by comments. Each kind states
its own ChangeEntityType instead of casting: the two enums agree numerically
but do not even share member names (Host against SshHost), and filing key
changes under the host type is silent sync corruption — sabotaging it fails
three tests. And EntityTypeAlignmentTests asserts the two enums stay aligned
in both directions and in count, which nothing did before.

The client half is next: SshKeySecret, its codec and merge, the cipher, a
repository, and the UI. Note for that work — SyncEntityType.SshKey is 3 while
AadResourceType.SshKey is 6, so a cast between them would seal key ciphertext
as a vault and nothing would fail.
2026-07-29 15:14:06 +02:00
jaap-jan 3829217e8a Add sync engine: cursors, push/pull, and the advisory-lock ordering proof (M1)
The vault write path. Push is the only way items change — no per-entity POST/PUT/DELETE —
so one place enforces revisions, the change log and access control.

The concurrency hazard, now proven rather than asserted:
bigserial assigns sequence values when the INSERT runs, not at commit, so transaction A
can take sequence 5 while B takes 6 and commits first. A reader polling in between sees
only 6, advances past 5, and never learns about it. AdvisoryLockOrderingTests reproduces
that gap WITHOUT the lock first — otherwise the with-lock test proves nothing, since it
would pass just as happily if the interleaving never occurred — then shows
pg_advisory_xact_lock removes it, and that 12 concurrent writers produce no gaps.

Cursors are opaque and HMAC-tagged, and carry their vault id. 29 unit tests cover the
rejections, which are the point: an accepted-but-wrong cursor is silent data loss, strictly
worse than an error a client can resync from. Rejected: tampered tag, tampered payload,
foreign signing key, a legitimately-issued cursor from another vault, truncation, and
hostile input (never throws — cursors come from clients).

Push semantics:
- 200 even on partial failure, with per-operation status, so one stale item cannot block
  everything a client queued while offline.
- Conflict returns the server's current row for client-side three-way merge. The server
  cannot merge ciphertext, so never last-writer-wins.
- opId receipts make retries exactly-once per operation, not per batch — a client retrying
  a partially-overlapping batch after a timeout would otherwise double-apply what landed.
- A tombstone beats a late upsert, and delete clears hostname/port: leaving the address
  would keep the server able to resolve a host the user believes they deleted.
- Relay field validation mirrors the DB CHECK so a bad request is a clear Invalid rather
  than a constraint violation surfacing as a 500.

Authorization goes through IVaultAccessService, which returns the same answer for "absent"
and "forbidden" — distinguishing them is an existence oracle for other tenants' vault ids.
Team vaults are explicitly denied until M3 rather than falling through to a permissive
default. JIT provisioning keys on (issuer, subject), never email, and handles the
concurrent-first-request race via the unique index.

Renamed two domain types: Host -> SshHost, because Host collides with
Microsoft.Extensions.Hosting.Host in every file of a web project, and SyncChange ->
VaultChange to stop it colliding with the Contracts DTO of the same name. Aliasing at every
use site would have been permanent friction.

Worth noting: `ef migrations has-pending-model-changes` reported clean after those renames
even though the snapshot still said "DodoSSH.Domain.Host" — it diffs tables, not CLR type
names. The snapshot was regenerated and the emitted DDL diffed against the previous
artifacts/schema/v0.1.sql to confirm the rename produced no schema change.

Also removed ConfigureAwait(false) from test methods: xUnit1030 flags it as bypassing
parallelization limits, which is why MA0004 is suppressed in test projects.

Verified: 0 warnings on a clean rebuild, 146 tests pass (up from 122), format clean.

Endpoint-level tests are the immediate next step: they need a WireMock OIDC/JWKS stub and
real JWT minting, so the "wrong user is denied" matrix does not exist yet for these two
routes. The service-layer authorization and the concurrency property are covered.
2026-07-28 15:02:02 +02:00
jaap-jan eaf68c86b0 Add data model, DbContext and initial migration (M1)
Schema for identity, vaults, grants, hosts and the sync change log, verified against a
real PostgreSQL 18 container rather than an in-memory provider: partial unique indexes,
CHECK constraints, citext and identity-always columns are all provider behaviour that an
in-memory fake would not exercise.

Invariants pushed into the database, so they hold even when application code has a bug:
- ck_host_relay_target is a security boundary, not tidiness. A host may carry a plaintext
  hostname and port ONLY when relay is deliberately enabled. Both directions are tested;
  the important one is that relay-disabled hosts cannot carry an address, since otherwise
  a bug would silently give the server infrastructure visibility it was never granted.
- ck_vault_owner: exactly one of owner_user_id or team_id, or permission resolution would
  have no defined answer.
- ck_vault_key_grant_recipient: member grants name a user; recovery and escrow grants are
  wrapped to a key and must not.
- ck_user_key_wrap_kdf: a password-derived wrap without its parameters is permanently
  unopenable, so a partial write is rejected outright.

Present from the first migration on purpose:
- GrantKind (Member/Recovery/Escrow). Recovery cannot be bolted on later — every vault
  created before it existed would be unrecoverable by design.
- team and team_membership, though team features are M3. Adding them later would mean
  introducing a foreign key on a live vault table.
- Host.ContentKeyId, reserved for per-item content keys wrapped to individual users.
- user_key as its own table, so key rotation does not require altering the user row.

Two things verified rather than assumed:
- Npgsql's UseXminAsConcurrencyToken helper no longer exists in EF 10, so xmin is mapped
  directly in XminConcurrency. The generated migration *looks* like it creates an xmin
  column; it does not. Confirmed by inspecting pg_attribute (attnum -2, a system column)
  and by grepping the emitted DDL. A test pins both, because had it created a real column
  PostgreSQL would have rejected the name.
- EF Core is now pinned centrally. The Npgsql provider asks for 10.0.4 while
  EntityFrameworkCore.Design pulls 10.0.10, and because Design is PrivateAssets=all that
  higher version does not flow to referencing projects — producing a CS1705 in any test
  project referencing Infrastructure.

Also commits artifacts/schema/v0.1.sql, the idempotent script, as the baseline for future
upgrade tests.

Verified: 0 warnings, 122 tests pass (27 new against Postgres), format clean.
2026-07-28 14:17:37 +02:00