Shares gain an addressee: store, authorisation, read path (#529, 1/3) #570

Closed
opened 2026-09-17 01:14:01 +00:00 by Cordy · 4 comments
Owner

First slice of #529, backend only and invisible until 2/3 lands. Approved from sharing-rework-529-mockup.html rev 1.

The decision this implements

The grant authorises; the path still keys. encrypt/group.go derives the key scope from the backend path — /spaces/<name>/ uses that space's group recipient, everything else the owning user — so an addressee on a share record cannot make a recipient able to decrypt. It makes them entitled. The read then goes through the owner's key scope, which is the mechanism the public-link handler already uses to serve unauthenticated callers.

Rejected alternatives, recorded so they are not revisited by accident: re-encrypting on every share (rewrites content on every membership change, collides with legal holds #138, versioning #541 and quota) and leaving sharing as space membership only (the divergence from OpenCloud that #529 exists to close).

Scope

  • share.Share gains an addressee — a kind (user | group) and a name, alongside the existing path-addressed link fields. A record carries either an addressee or link policy, never both; the shape should make the invalid state unrepresentable rather than validated after the fact.
  • Role on the grant: read | write, reusing spacestore's vocabulary. AllowUpload stays as it is for folder file-drop.
  • Authorisation resolves through User.Groups, which since #550 already merges app-owned and directory groups — so app groups become share targets with no extra plumbing.
  • Read path: an entitled recipient's request opens the object in the owner's key scope. This is the one genuinely new piece and wants the most careful tests.
  • Audit attribution: a group share attributes to the acting person, not to the group. This is the reason people-shares are worth having at all (link access records as a link, not a person), so it is not optional polish.
  • Fail-closed resolution: an addressee that no longer resolves grants nothing, mutates nothing, and is reportable. Never a silent cascade — see nextcloud/server#25755 for the failure mode being avoided.
  • Store.List gains a way to query by addressee and by path, which is what #550's Used-by count and 2/3's panel both need.

Out of scope here

The panel (2/3), federated relocation (3/3), owner-deletion semantics (#569), re-sharing by recipients (decided: no).

Verifiable done

go test ./... green with tests covering: a user grant admits and a non-grantee is refused; a group grant admits via app-owned and directory group membership; a revoked grant refuses immediately with no re-encryption anywhere; an unresolvable addressee grants nothing; audit records the person for a group share; and the read path serves an entitled recipient a file encrypted to the owner's scope. House TDD — red witnessed on the runner before implementation.

First slice of #529, backend only and invisible until 2/3 lands. Approved from `sharing-rework-529-mockup.html` rev 1. ## The decision this implements **The grant authorises; the path still keys.** `encrypt/group.go` derives the key scope from the backend path — `/spaces/<name>/` uses that space's group recipient, everything else the owning user — so an addressee on a share record cannot make a recipient able to *decrypt*. It makes them *entitled*. The read then goes through the **owner's key scope**, which is the mechanism the public-link handler already uses to serve unauthenticated callers. Rejected alternatives, recorded so they are not revisited by accident: re-encrypting on every share (rewrites content on every membership change, collides with legal holds #138, versioning #541 and quota) and leaving sharing as space membership only (the divergence from OpenCloud that #529 exists to close). ## Scope - **`share.Share` gains an addressee** — a kind (`user` | `group`) and a name, alongside the existing path-addressed link fields. A record carries either an addressee or link policy, never both; the shape should make the invalid state unrepresentable rather than validated after the fact. - **Role** on the grant: `read` | `write`, reusing `spacestore`'s vocabulary. `AllowUpload` stays as it is for folder file-drop. - **Authorisation resolves through `User.Groups`**, which since #550 already merges app-owned *and* directory groups — so app groups become share targets with no extra plumbing. - **Read path**: an entitled recipient's request opens the object in the owner's key scope. This is the one genuinely new piece and wants the most careful tests. - **Audit attribution**: a group share attributes to the *acting person*, not to the group. This is the reason people-shares are worth having at all (link access records as a link, not a person), so it is not optional polish. - **Fail-closed resolution**: an addressee that no longer resolves grants nothing, mutates nothing, and is reportable. Never a silent cascade — see nextcloud/server#25755 for the failure mode being avoided. - **`Store.List`** gains a way to query by addressee and by path, which is what #550's Used-by count and 2/3's panel both need. ## Out of scope here The panel (2/3), federated relocation (3/3), owner-deletion semantics (#569), re-sharing by recipients (decided: no). ## Verifiable done `go test ./...` green with tests covering: a user grant admits and a non-grantee is refused; a group grant admits via app-owned *and* directory group membership; a revoked grant refuses immediately with no re-encryption anywhere; an unresolvable addressee grants nothing; audit records the person for a group share; and the read path serves an entitled recipient a file encrypted to the owner's scope. House TDD — red witnessed on the runner before implementation.
Cordy referenced this issue from a commit 2026-09-17 01:18:19 +00:00
Author
Owner

The body above claims the read path uses "the owner's key scope, which is the mechanism the public-link handler already uses to serve unauthenticated callers." That is wrong, and the same wrong sentence is in #529's mockup and in the chat that produced it. Recording it here rather than quietly editing.

What the public handler actually does:

type PublicHandler struct {
	Store  Store
	Driver storage.Driver
	// RecoveryOpen (#175): opens a file's plaintext using the deployment
	// recovery identity when the per-user-custody Open fails on this
	// anonymous surface. Feature-detected; nil on unencrypted instances
	// and remote-crypto splits.
	RecoveryOpen func(ctx context.Context, path string) (io.ReadSeekCloser, error)
	// AuditRead records recovery-path reads (share token + path).
	AuditRead func(shareToken, path string)
}

It reads via the deployment recovery identity, not the owner's, as a documented feature-detected fallback, separately audited as a recovery-path read. It also takes a raw storage.Driver with absolute backend paths, because it mounts outside the auth middleware.

The approved decision is unaffected — grant authorises, path keys, the server opens the owner's file for the entitled recipient. Only my description of the existing plumbing was inaccurate. But the difference matters, because the scope driver's resolve() maps /home to /home/<requesting user> with no admin exemption (#177), so there is no existing way for Bob to address Alice's home. A new seam is needed either way.

Mechanism decided (Nikola, 2026-09-17)

Owner-scope open, plus a space shortcut.

  • Personal-tree shares get a new narrow seam: resolve the owner from the share record, build a context with the owner as principal, and read through the normal per-user custody path. Audited as a share read attributed to the acting person — which is the entire reason people-shares beat links.
  • Group shares on space paths need nothing new: the recipient is already a member, so the ordinary scope path resolves. Only personal-tree shares require the seam.
  • RecoveryOpen is left alone. Reusing it would have been cheaper, but every shared read would then log as a recovery-path read — blurring a signal that currently means something specific, and it is nil on unencrypted and remote-crypto deployments, where internal sharing would have silently failed.

The "Verifiable done" list above stands, with one addition: a test asserting that a share read does not invoke the recovery path, so the two mechanisms cannot quietly merge later.

## Correction: the public-link precedent is not what this issue says The body above claims the read path uses "the owner's key scope, which is the mechanism the public-link handler already uses to serve unauthenticated callers." **That is wrong**, and the same wrong sentence is in #529's mockup and in the chat that produced it. Recording it here rather than quietly editing. What the public handler actually does: ```go type PublicHandler struct { Store Store Driver storage.Driver // RecoveryOpen (#175): opens a file's plaintext using the deployment // recovery identity when the per-user-custody Open fails on this // anonymous surface. Feature-detected; nil on unencrypted instances // and remote-crypto splits. RecoveryOpen func(ctx context.Context, path string) (io.ReadSeekCloser, error) // AuditRead records recovery-path reads (share token + path). AuditRead func(shareToken, path string) } ``` It reads via the **deployment recovery identity**, not the owner's, as a documented feature-detected fallback, separately audited as a recovery-path read. It also takes a raw `storage.Driver` with absolute backend paths, because it mounts outside the auth middleware. **The approved decision is unaffected** — grant authorises, path keys, the server opens the owner's file for the entitled recipient. Only my description of the existing plumbing was inaccurate. But the difference matters, because the scope driver's `resolve()` maps `/home` to `/home/<requesting user>` with no admin exemption (#177), so there is no existing way for Bob to address Alice's home. A new seam is needed either way. ## Mechanism decided (Nikola, 2026-09-17) **Owner-scope open, plus a space shortcut.** - **Personal-tree shares** get a new narrow seam: resolve the owner from the share record, build a context with the owner as principal, and read through the normal per-user custody path. Audited as a *share read attributed to the acting person* — which is the entire reason people-shares beat links. - **Group shares on space paths** need nothing new: the recipient is already a member, so the ordinary scope path resolves. Only personal-tree shares require the seam. - **`RecoveryOpen` is left alone.** Reusing it would have been cheaper, but every shared read would then log as a recovery-path read — blurring a signal that currently means something specific, and it is `nil` on unencrypted and remote-crypto deployments, where internal sharing would have silently failed. The "Verifiable done" list above stands, with one addition: **a test asserting that a share read does not invoke the recovery path**, so the two mechanisms cannot quietly merge later.
Cordy referenced this issue from a commit 2026-09-17 01:22:43 +00:00
Author
Owner

Slice 1 landed — #573 merged.

Share now carries AddresseeKind / Addressee / Role / ID; Entitles, Key, ListForPath, ListForAddressee and RoleForPath are implemented and tested. Backend only — nothing is reachable or visible yet.

Three decisions recorded here because they constrain the next slice:

1. Store was not widened; the grant queries live on GrantQuerier.
My first green attempt added ListForPath / ListForAddressee to the Store interface. That broke recStore in public_recovery_test.go — a link-only double that exists to prove the #175 recovery fallback and will never answer a grant query. Widening Store would force every link-only implementation to grow two methods it cannot meaningfully answer. GrantQuerier is a separate two-method interface; *FileStore satisfies both, PublicHandler keeps taking Store. Any future SQLite backend implements GrantQuerier only if it wants to serve grants.

2. Path match is exact, never prefix.
A grant on /home/alice/reports does not reach /home/alice/reports-private. Folder sharing is its own feature with its own semantics, and prefix inheritance is a data leak that reads like a typo. Fenced by TestRoleForPathIsExact.

3. The audit verbs are deferred, deliberately.
The scope sentence for this slice named "the audit verb". It is not in #573, and that is a decision rather than an omission: share-grant and share-grant-revoke have no emit site until the grant create/revoke API exists, and the entitled-read verb has none until the read path is wired. internal/audit/verbs.go is append-only by design (#433) — a verb registered ahead of its feature is a permanent name chosen before the feature that names it. verbs_test.go only fails on emitted-but-unregistered, so nothing forces the order. They land with the code that emits them, in the next slice.

One implementation note worth keeping: grants have no token, so FileStore re-keys through Key() (g:<id> for a grant, the token for a link). Every write-side access was re-keyed — Create, the Create rollback, and the load path in OpenFileStore. Get and Delete still key by token, which is correct: they are link lookups. Missing the load path alone would have collapsed every grant onto "" on reload — silently, and only in the persisted file.

Next: /shared tree + owner-scope open, per the approved split.

**Slice 1 landed — #573 merged.** `Share` now carries `AddresseeKind` / `Addressee` / `Role` / `ID`; `Entitles`, `Key`, `ListForPath`, `ListForAddressee` and `RoleForPath` are implemented and tested. Backend only — nothing is reachable or visible yet. Three decisions recorded here because they constrain the next slice: **1. `Store` was not widened; the grant queries live on `GrantQuerier`.** My first green attempt added `ListForPath` / `ListForAddressee` to the `Store` interface. That broke `recStore` in `public_recovery_test.go` — a link-only double that exists to prove the #175 recovery fallback and will never answer a grant query. Widening `Store` would force every link-only implementation to grow two methods it cannot meaningfully answer. `GrantQuerier` is a separate two-method interface; `*FileStore` satisfies both, `PublicHandler` keeps taking `Store`. Any future SQLite backend implements `GrantQuerier` only if it wants to serve grants. **2. Path match is exact, never prefix.** A grant on `/home/alice/reports` does not reach `/home/alice/reports-private`. Folder sharing is its own feature with its own semantics, and prefix inheritance is a data leak that reads like a typo. Fenced by `TestRoleForPathIsExact`. **3. The audit verbs are deferred, deliberately.** The scope sentence for this slice named "the audit verb". It is not in #573, and that is a decision rather than an omission: `share-grant` and `share-grant-revoke` have no emit site until the grant create/revoke API exists, and the entitled-read verb has none until the read path is wired. `internal/audit/verbs.go` is append-only by design (#433) — a verb registered ahead of its feature is a permanent name chosen before the feature that names it. `verbs_test.go` only fails on *emitted-but-unregistered*, so nothing forces the order. They land with the code that emits them, in the next slice. One implementation note worth keeping: grants have no token, so `FileStore` re-keys through `Key()` (`g:<id>` for a grant, the token for a link). Every **write-side** access was re-keyed — `Create`, the `Create` rollback, and the load path in `OpenFileStore`. `Get` and `Delete` still key by token, which is correct: they are link lookups. Missing the load path alone would have collapsed every grant onto `""` on reload — silently, and only in the persisted file. **Next:** `/shared` tree + owner-scope open, per the approved split.
Author
Owner

Slice 2 landed — #574 merged. /shared exists, resolves, and reads through the owner's key scope.

Four decisions taken this round, three of them Nikola's:

1. Layout is /shared/<owner>/<name>, not flat. Two people can each share a report.pdf; a flat tree has to mangle one of the names, and "who gave me this" stops being visible without opening a details pane.

2. Directory grants are in, and carry their subtree. Matched segment by segment, never by string prefix — so reports still does not reach reports-private. That distinction is the whole reason slice 1's RoleForPath is exact, and TestSharedSiblingPrefixIsNotReachable is the fence for it here.

3. Write-role grants are writable now. The write re-encrypts into the owner's scope, because that is the only scope the file can live in. The consequence is that ownermeta will attribute the change to the owner rather than the editor — accepted knowingly rather than discovered later. If that turns out to matter in the beta it is a separate fix, in ownermeta, not here.

4. The audit hook sits in the scope driver, above the identity swap. This is the one that would have gone wrong silently. Everything below the scope decorator sees the owner, so an audit record made down there reads "alice read her own file" — and an entitled access becomes indistinguishable from ordinary activity, which is the exact case the record exists for. The hook takes the real reader and the owner separately, and TestSharedAuditNamesTheReaderNotTheOwner asserts both halves at once: the hook saw Bob, the inner driver saw Alice. Neither can drift without the test failing.

share-grant-read and share-grant-write are now registered in internal/audit/verbs.go, which closes the deferral I recorded when slice 1 landed.

Deliberately not wired. WithGrants is not called in cmd/cairnd/main.go. Nothing can create a grant yet, so wiring it would ship a root that can only ever be empty. The adapter from the share store to scope.Grants lands with #571, alongside the panel and the create/revoke API.

#570 remaining: the grant create/revoke API (with its own two audit verbs), then the adapter + wiring. Both belong with #571 rather than here, since the panel is what calls them.

**Slice 2 landed — #574 merged.** `/shared` exists, resolves, and reads through the owner's key scope. Four decisions taken this round, three of them Nikola's: **1. Layout is `/shared/<owner>/<name>`, not flat.** Two people can each share a `report.pdf`; a flat tree has to mangle one of the names, and "who gave me this" stops being visible without opening a details pane. **2. Directory grants are in, and carry their subtree.** Matched segment by segment, never by string prefix — so `reports` still does not reach `reports-private`. That distinction is the whole reason slice 1's `RoleForPath` is exact, and `TestSharedSiblingPrefixIsNotReachable` is the fence for it here. **3. Write-role grants are writable now.** The write re-encrypts into the owner's scope, because that is the only scope the file can live in. The consequence is that `ownermeta` will attribute the change to the owner rather than the editor — accepted knowingly rather than discovered later. If that turns out to matter in the beta it is a separate fix, in `ownermeta`, not here. **4. The audit hook sits in the scope driver, above the identity swap.** This is the one that would have gone wrong silently. Everything below the scope decorator sees the owner, so an audit record made down there reads "alice read her own file" — and an entitled access becomes indistinguishable from ordinary activity, which is the exact case the record exists for. The hook takes the real reader and the owner separately, and `TestSharedAuditNamesTheReaderNotTheOwner` asserts both halves at once: the hook saw Bob, the inner driver saw Alice. Neither can drift without the test failing. `share-grant-read` and `share-grant-write` are now registered in `internal/audit/verbs.go`, which closes the deferral I recorded when slice 1 landed. **Deliberately not wired.** `WithGrants` is not called in `cmd/cairnd/main.go`. Nothing can create a grant yet, so wiring it would ship a root that can only ever be empty. The adapter from the share store to `scope.Grants` lands with #571, alongside the panel and the create/revoke API. **#570 remaining:** the grant create/revoke API (with its own two audit verbs), then the adapter + wiring. Both belong with #571 rather than here, since the panel is what calls them.
Author
Owner

Closing — all three parts of the scope are in and live.

Scope Where it landed
Store #573Share gains addressee/role/id, FileStore keys grants separately from links
Authorisation #573Entitles, RoleForPath, exact-path matching
Read path #574 — the /shared overlay, owner-scope open, reader-attributed audit
Audit verbs #574 (share-grant-read/-write), #575 (share-grant/-revoke)
Create/revoke API #575, filed under #571 because the panel is what calls it

The three decisions from this issue that the rest of the feature now rests on:

  1. Store was not widened. The grant queries live on GrantQuerier, so a link-only implementation is not forced to grow methods it cannot answer.
  2. Path matching is exact, never a string prefix. A grant on reports does not reach reports-private. Directory grants descend segment by segment, which preserves that.
  3. The audit hook sits above the identity swap. Everything below the scope decorator sees the owner, so a record made there would have read "alice read her own file" and made an entitled access indistinguishable from ordinary activity.

Remaining sharing work is tracked on #571 (panel + admin toggle) and #572 (federated send relocation).

**Closing — all three parts of the scope are in and live.** | Scope | Where it landed | |---|---| | Store | #573 — `Share` gains addressee/role/id, `FileStore` keys grants separately from links | | Authorisation | #573 — `Entitles`, `RoleForPath`, exact-path matching | | Read path | #574 — the `/shared` overlay, owner-scope open, reader-attributed audit | | Audit verbs | #574 (`share-grant-read`/`-write`), #575 (`share-grant`/`-revoke`) | | Create/revoke API | #575, filed under #571 because the panel is what calls it | The three decisions from this issue that the rest of the feature now rests on: 1. **`Store` was not widened.** The grant queries live on `GrantQuerier`, so a link-only implementation is not forced to grow methods it cannot answer. 2. **Path matching is exact, never a string prefix.** A grant on `reports` does not reach `reports-private`. Directory grants descend segment by segment, which preserves that. 3. **The audit hook sits above the identity swap.** Everything below the scope decorator sees the owner, so a record made there would have read "alice read her own file" and made an entitled access indistinguishable from ordinary activity. Remaining sharing work is tracked on #571 (panel + admin toggle) and #572 (federated send relocation).
Cordy closed this issue 2026-09-17 12:20:17 +00:00
Sign in to join this conversation.
No labels
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Cordy/Cairn#570
No description provided.