Cordy/Cairn

Fork 0

3 follow-up) #588

New issue

Closed

opened 2026-09-18 00:48:12 +00:00 by Cordy · 1 comment

Cordy commented

2026-09-18 00:48:12 +00:00

Owner

Found in the v0.6.213 live pass, signed in as the recipient of a Can view grant.

What happens

nikola-test granted /home/nikola-test/moverini to nikola-test2 with role read. As nikola-test2, opening #/shared/nikola-test/moverini shows the breadcrumb / shared / nikola-test / moverini with a View only badge — correct — and the folder contents decrypt correctly, which is #570's overlay doing its job.

The row menu on a file inside that folder offers, in order:

Open · Download · Share · Copy link · Send to peer · Copy to… · Move · Rename · Create space from selection · Add to favorites · Lock · Details · Delete

Everything in bold is either a write to someone else's file or a re-share of it. The toolbar above also still shows Upload file and New.

Why it matters

The backend is not the problem. scope.Driver.Delegated answers "was this path reached through someone else's grant" and the re-share API refuses before writing anything — #575 landed that with a test asserting both the 403 and that the store stayed empty. Role enforcement on writes sits behind the same scope.

So the failure is presentational, and that is exactly what makes it worth fixing rather than shrugging at: the UI invites a recipient to do eight things the server will refuse, two words away from a badge that says View only. The user learns the product is broken, when in fact the product is correct and the menu is lying. Delete rendering in danger red on a file you do not own is the sharpest version of it.

This is also the fourth failure in this slice with the same shape — correct logic, wrong surface. #580 (switch never reached the page), #582 (grant rendered as a link), #586 (save bar never appeared), now this.

What the fix needs to decide

Two separate gates, and they are not the same gate:

Delegation. Actions that re-share or re-address someone else's file — Share, Copy link, Send to peer, Create space from selection — should be absent whenever Delegated is true, at any role. A recipient with write still must not become a second grantor; that was decided in #571 and made structural in #575.
Role. Mutating actions — Move, Rename, Delete, Lock, and the toolbar's Upload file / New — should follow the grant's role: absent on read, present on write. AllowUpload governs upload specifically, and that role does not exist yet — see #589.

Worth confirming while implementing: whether Add to favorites and Details are safe on a delegated read path (they look local-only, but check), and whether Copy to… is a read of the source plus a write to the caller's own tree — if so it is legitimate on read and should stay.

Suggested test

The house pattern from web/test/settings-load.test.js fits: lift the row-menu builder, feed it a delegated read scope, and assert the built menu contains none of the forbidden verbs. Pure list-of-labels assertion, no DOM needed. Add the write and non-delegated cases so the gate cannot be satisfied by hiding everything.

Reproduce

As user A, share a folder with user B, role Can view.
As user B, Shares → Shared with me → open it.
Open any file's row menu.

Found in the v0.6.213 live pass, signed in as the **recipient** of a `Can view` grant. ## What happens `nikola-test` granted `/home/nikola-test/moverini` to `nikola-test2` with role `read`. As `nikola-test2`, opening `#/shared/nikola-test/moverini` shows the breadcrumb `/ shared / nikola-test / moverini` with a **View only** badge — correct — and the folder contents decrypt correctly, which is #570's overlay doing its job. The row menu on a file inside that folder offers, in order: `Open` · `Download` · **`Share`** · `Copy link` · **`Send to peer`** · `Copy to…` · **`Move`** · **`Rename`** · **`Create space from selection`** · `Add to favorites` · **`Lock`** · `Details` · **`Delete`** Everything in bold is either a write to someone else's file or a re-share of it. The toolbar above also still shows **Upload file** and **New**. ## Why it matters The backend is not the problem. `scope.Driver.Delegated` answers "was this path reached through someone else's grant" and the re-share API refuses before writing anything — #575 landed that with a test asserting both the 403 and that the store stayed empty. Role enforcement on writes sits behind the same scope. So the failure is presentational, and that is exactly what makes it worth fixing rather than shrugging at: the UI invites a recipient to do eight things the server will refuse, two words away from a badge that says **View only**. The user learns the product is broken, when in fact the product is correct and the menu is lying. `Delete` rendering in danger red on a file you do not own is the sharpest version of it. This is also the fourth failure in this slice with the same shape — correct logic, wrong surface. #580 (switch never reached the page), #582 (grant rendered as a link), #586 (save bar never appeared), now this. ## What the fix needs to decide Two separate gates, and they are not the same gate: 1. **Delegation.** Actions that re-share or re-address someone else's file — `Share`, `Copy link`, `Send to peer`, `Create space from selection` — should be absent whenever `Delegated` is true, at any role. A recipient with `write` still must not become a second grantor; that was decided in #571 and made structural in #575. 2. **Role.** Mutating actions — `Move`, `Rename`, `Delete`, `Lock`, and the toolbar's `Upload file` / `New` — should follow the grant's role: absent on `read`, present on `write`. `AllowUpload` governs upload specifically, and that role does not exist yet — see **#589**. Worth confirming while implementing: whether `Add to favorites` and `Details` are safe on a delegated read path (they look local-only, but check), and whether `Copy to…` is a read of the source plus a write to the caller's own tree — if so it is legitimate on `read` and should stay. ## Suggested test The house pattern from `web/test/settings-load.test.js` fits: lift the row-menu builder, feed it a delegated read scope, and assert the built menu contains none of the forbidden verbs. Pure list-of-labels assertion, no DOM needed. Add the `write` and non-delegated cases so the gate cannot be satisfied by hiding everything. ## Reproduce 1. As user A, share a folder with user B, role `Can view`. 2. As user B, Shares → Shared with me → open it. 3. Open any file's row menu.

Cordy added this to the v0.7 — Closed beta (dogfood + hardening) milestone

2026-09-18 00:48:12 +00:00

Cordy referenced this issue

2026-09-18 00:48:28 +00:00

The "Can upload" role was never built — share dialog offers only view and edit (#529 2/3 follow-up) #589

Cordy referenced this issue

2026-09-18 00:49:15 +00:00

Share panel: people and groups first, links split internal/public (#529, 2/3) #571

Cordy referenced this issue

2026-09-18 00:50:49 +00:00

Docs run: the handbook still says a share is a link (#529 2/3, shipped v0.6.212) #585

Cordy referenced this issue

2026-09-18 01:20:54 +00:00

docs(#585): the handbook stops saying a share is a link #591

Cordy referenced this issue from a commit

2026-09-18 01:27:39 +00:00

chore: one-shot for the #588 fix (red then green on fix-588)

forgejo-actions referenced this issue from a commit

2026-09-18 01:39:07 +00:00

fix(#588): the row menu offers only what the grant allows

Cordy referenced this issue

2026-09-18 01:40:07 +00:00

fix(#588): the row menu offers only what the grant allows #593

Cordy referenced this issue from a commit

2026-09-18 01:43:43 +00:00

fix(#588): the row menu offers only what the grant allows (#593)

Cordy referenced this issue

2026-09-18 01:46:24 +00:00

docs: v0.6.214 changelog entry #594

Cordy referenced this issue

2026-09-18 02:01:16 +00:00

Decide: any space member can grant space content outward #595

Cordy commented

2026-09-18 02:01:29 +00:00

Author

Owner

Fixed in #593, shipped as v0.6.214, live on both dogfoods.

Two gates, as the issue asked, kept distinct: share (false anywhere under /shared — re-sharing and re-addressing never offered at any role) and mutate (false when the listing carries the #64 readOnly flag — Can-view recipients and read-only space members alike). Copy to… survives every role, as anticipated: it reads the source and writes only to the caller's own tree.

Two findings from implementation worth the record:

The toolbar was never the bug. applyReadOnly (#64) has disabled Upload/New on read-only listings all along — but the upload control is a label wrapping the file input and nothing styled the disabled state, so it looked live (the #552 lesson, again). One .rodis class fixes the look; behaviour was already correct.
The read-only-space gap came free. Because mutate rides the same server flag #64 already sets for read-only space members, they also stop being offered Move/Rename/Delete/Lock — a gap that predates #571 entirely.

Coverage is the glue, not just the mapping: web/test/row-menu.test.js lifts openRowMenu and builds the real menu against a fake DOM in four scenarios (delegated read, delegated write, owned, read-only space), plus a pure truth-table for menuGates. Red witnessed on all five before the splice.

Remaining: an eyeball on the dogfood as the recipient — sessions are in-memory, so the v0.6.214 rollout needs a fresh login first. The nikola-test → nikola-test2 grant on moverini is still in place for exactly that.

**Fixed in #593, shipped as v0.6.214, live on both dogfoods.** Two gates, as the issue asked, kept distinct: `share` (false anywhere under `/shared` — re-sharing and re-addressing never offered at any role) and `mutate` (false when the listing carries the #64 `readOnly` flag — Can-view recipients and read-only space members alike). `Copy to…` survives every role, as anticipated: it reads the source and writes only to the caller's own tree. Two findings from implementation worth the record: - **The toolbar was never the bug.** `applyReadOnly` (#64) has disabled Upload/New on read-only listings all along — but the upload control is a `label` wrapping the file input and nothing styled the disabled state, so it *looked* live (the #552 lesson, again). One `.rodis` class fixes the look; behaviour was already correct. - **The read-only-space gap came free.** Because `mutate` rides the same server flag #64 already sets for read-only space members, they also stop being offered Move/Rename/Delete/Lock — a gap that predates #571 entirely. Coverage is the glue, not just the mapping: `web/test/row-menu.test.js` lifts `openRowMenu` and builds the real menu against a fake DOM in four scenarios (delegated read, delegated write, owned, read-only space), plus a pure truth-table for `menuGates`. Red witnessed on all five before the splice. Remaining: an eyeball on the dogfood as the recipient — sessions are in-memory, so the v0.6.214 rollout needs a fresh login first. The `nikola-test → nikola-test2` grant on `moverini` is still in place for exactly that.

Cordy closed this issue

2026-09-18 02:01:40 +00:00

Cordy referenced this issue

2026-09-18 02:02:14 +00:00

Share panel: people and groups first, links split internal/public (#529, 2/3) #571

Cordy referenced this issue

2026-09-18 02:38:24 +00:00

Departed owners: freeze, surface, transfer (#569 implementation) #597

Cordy referenced this issue

2026-09-18 02:58:33 +00:00

fix(#595): grants refuse space paths — membership is the grant #600

Cordy referenced this issue