Public Access
Merge branch 'main' into claude/host-management-ui-plan-7f20ab
Seven files needed a hand. Most were two branches adding something in the same place, but three were one branch changing what the other had moved or renamed, and those are the ones worth reading. The shell keeps both new fields and both constructor lines: the connection recorder this branch built and the teams view model main did. Where main put a teams load inside OnScreenChanged, it now sits beside the logs refresh rather than inside RaiseSurfaceState — this branch extracted that notification block and it is called from two properties, so a screen-specific side effect in there would fire on every terminal switch as well. Main gave four row types a vault id and a vault name, and this branch had moved one of them — KnownHostRowViewModel — into its own file when the pinned keys became a screen. Git resolved that as "deleted here, modified there" and took the delete, which compiles as long as nobody looks: the moved copy still had the two-argument constructor and the call site had grown to four. Carried over by hand, along with the ordering the pins list now does on them. The status line's quiet rule was the subtle one. Main extracted it into IsWorthReporting; this branch had changed the same condition to read item counts rather than raw ones, because every user action queues a log entry a moment later and this machine reads its own entries back on the next pull. Take main's structure and the merge builds, passes, and silently restores a bug this branch existed partly to fix — every save's message overwritten a second after it appears. The method now reads PulledItems and PushedItems, with the reason in its remarks. Two conflicts were prose that had gone stale rather than code. The keychain screen's comment said team vaults are refused by the server's access service, which was true when it was written and is not now; main's replacement stands, in this branch's vocabulary. The design-gaps row for groups was claimed by both — real host groups here, per-vault headings there — and they are different things, so both rows stay and the difference is stated: a group is a shelf the user chose, a vault is who can read the item. One defect the tests found and the compiler could not. Generating a key opens the same editor as pasting one, but not through NewKey — so it never set the target vault main added, and a generated key was filed into whatever vault was edited last, or none. Both key-generation tests failed on it. Fixed where the editor opens, with the reason recorded there. One gap is left deliberately and is written down rather than half-built. Hosts, keys, credentials and pins are read across every vault this session holds a key for; groups are read from the active vault alone, so a host a teammate filed shows under UNGROUPED. Nothing is lost or misfiled — it is what the sidebar already shows for a group that has been deleted — but closing it needs a vault id on every group row for rename and delete, and a way to tell two vaults' identically-named groups apart under a layout with one heading per group. Both are worth doing and neither is a merge's business. It is in the remarks on ReloadGroupsAsync and in docs/design-import-gaps.md. dotnet build, dotnet test and dotnet format --verify-no-changes are all clean: 1282 tests, including the end-to-end suite against real containers.
This commit is contained in:
@@ -0,0 +1,89 @@
|
||||
# ADR 0009 — Team access: membership authorises, a grant unlocks
|
||||
|
||||
- Status: accepted
|
||||
- Date: 2026-07-31
|
||||
- Builds on: [ADR 0001](0001-e2ee-trust-model.md)
|
||||
|
||||
## Context
|
||||
|
||||
M3 makes vaults shareable. The obvious way to model that is one concept — "access" — with a role
|
||||
attached, and to let the server hand it out. Every hosted competitor works that way, and it is what
|
||||
the imported design drew: a members table with a role column, and a share button beside each item.
|
||||
|
||||
This architecture cannot implement that concept, and the interesting part of M3 was working out
|
||||
what it can implement instead.
|
||||
|
||||
The server holds ciphertext and no keys. A vault key is 32 random bytes sealed to each member's
|
||||
X25519 public key (`docs/crypto.md` §3), and only a client holding the plaintext key can produce a
|
||||
seal for somebody else. So "give Bob access" decomposes into two operations that live on different
|
||||
machines and cannot be performed by the same actor:
|
||||
|
||||
- deciding that the server will **serve** Bob this vault's rows, which is a database write; and
|
||||
- **wrapping** the vault key to Bob's public key, which needs a client that already holds it.
|
||||
|
||||
The schema anticipated this — `team`, `team_membership`, `vault.team_id` and `vault_key_grant` have
|
||||
existed since the first migration — but nothing had had to name the split.
|
||||
|
||||
## Decision
|
||||
|
||||
**Membership is authorisation. A grant is access. The product says so out loud.**
|
||||
|
||||
`VaultAccessService` resolves a team vault through `team_membership`, mapping the role to
|
||||
`PermissionFlags` by a union with no Deny rules. That decides what the server serves and nothing
|
||||
else. Whether the caller can read what it serves is decided by whether they hold a grant, which the
|
||||
server records, cannot produce and cannot verify.
|
||||
|
||||
Four consequences, each of which is a place where a more reassuring design was rejected:
|
||||
|
||||
- **A member with no grant is a normal state, not an error.** `VaultSummary.WrappedVaultKey` is null
|
||||
and the vault appears in their list saying it is waiting for a key. Hiding it until a grant existed
|
||||
would have been tidier and would have implied the server was the thing granting access.
|
||||
- **The roles are only the ones that are enforceable.** There is no `ConnectOnly`, despite the design
|
||||
asking for one and `TeamRole` having room. SSH terminates on the client, so opening a session needs
|
||||
the credential's plaintext on that machine; "may connect but may not read the key" cannot be
|
||||
enforced here, and shipping it as a role would have been a lie in a dropdown. `Connect` rides along
|
||||
with `Read` and is documented as an interface hint.
|
||||
- **Sharing verifies the recipient's key against the append-only key log, or refuses.** A directory
|
||||
lookup is a claim by the server about a third party's public key; wrapping to an unverified claim
|
||||
hands the vault to whoever made it. `KeyLogAudit` reads the whole log, checks its hash chain from
|
||||
genesis, and refuses unless the offered key appears in it unchanged. There is no override flag,
|
||||
because a flag that exists gets used on the day the log is briefly unreachable.
|
||||
- **Removal is named for what it does.** It revokes grants and flags the vault for rekey. It does not
|
||||
claim to reach anything already downloaded, and the interface says the remediation is rotating the
|
||||
credential — the same non-retroactive limit ADR 0001 records.
|
||||
|
||||
Two things were deliberately **not** built, and both are refusals rather than omissions:
|
||||
|
||||
- **The rekey itself.** Only a client holding the current vault key can re-wrap every item's data key
|
||||
under a new one. The server records that a rotation is owed and the interface reports it. M5.
|
||||
- **Ownership transfer.** The owner cannot be demoted or removed, with its own problem code. Allowing
|
||||
it without a transfer would leave a team nobody can administer, recoverable only by an operator
|
||||
editing the database.
|
||||
|
||||
Two smaller choices, recorded because the alternative was written down first and rejected:
|
||||
|
||||
- **No `v_user_vault_permission` view.** ADR-adjacent notes and the old `VaultAccessService` remark
|
||||
both anticipated one. The rules turned out to be about sixteen lines of C# shared by the two
|
||||
methods that need them; a view would have moved the authorisation model into migrations, where a
|
||||
test cannot reach it without a container.
|
||||
- **Host key trust stays vault-scoped to the personal vault.** Pins in a team vault are listed but
|
||||
not consulted at connect time. Consulting them would let any member with Write pre-approve a
|
||||
fingerprint that another member's client then trusts silently for a host in their *own* vault,
|
||||
which is a cross-boundary trust escalation. Scoping trust properly needs a scope on the SSH connect
|
||||
path (`IKnownHostStore.FindAsync` takes host, port and algorithm and knows nothing about vaults);
|
||||
until that exists, the safe direction is the narrow one, and the cost — approving a team host's key
|
||||
once per member per machine — is stated in the README rather than hidden.
|
||||
|
||||
## Consequences
|
||||
|
||||
The sharing graph is visible to the operator: who is in which team, which vaults exist, and who holds
|
||||
a grant are all plaintext rows. That was already true of metadata generally (`docs/crypto.md` §10)
|
||||
and is not made worse here, but it is now a graph rather than a list.
|
||||
|
||||
A malicious granter can seal garbage. The recipient detects it as a tag failure and the grant's
|
||||
Ed25519 signature names who issued it — detectable and attributable, which is the most that is
|
||||
achievable without the server holding a key.
|
||||
|
||||
The two-step model costs a step in the interface and buys the property the whole product is for. It
|
||||
also makes a class of bug impossible: there is no code path on the server that could accidentally
|
||||
grant read access to plaintext, because there is no plaintext on the server to grant.
|
||||
Reference in New Issue
Block a user