Skip to content

🐛 PUT-1586 + PUT-1599: Share dialog says what it can grant, and what it did - #3647

Merged
jfcastro92 merged 2 commits into
juancastro/put-1590-sharing-shared-files-are-not-different-than-regular-filesfrom
juancastro/put-1586-1599-share-dialog-correctness
Aug 26, 2026
Merged

🐛 PUT-1586 + PUT-1599: Share dialog says what it can grant, and what it did#3647
jfcastro92 merged 2 commits into
juancastro/put-1590-sharing-shared-files-are-not-different-than-regular-filesfrom
juancastro/put-1586-1599-share-dialog-correctness

Conversation

@jfcastro92

@jfcastro92 jfcastro92 commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Two share-dialog bugs that share the same two files. Third in the stack — base is #3646, which is based on #3644. Review those first; this diff is only its own two commits.

Fixes PUT-1586 and PUT-1599.


PUT-1586 — a delegate could not pass on "can edit & share"

Reproduced against a running server before changing anything, and the ticket's "cannot share" turns out to be narrower than it reads: a recipient holding manage re-shares fine at read and write. Only manage fails.

delegate grants read   -> success
delegate grants write  -> success
delegate grants manage -> error: Forbidden

Refusing is deliberate — handing out manage needs authority over manage, which only the owner has, so delegation is one level deep by construction. The defects were around it: the dropdown offered the level, and the refusal was a bare 403 that reads as a bug.

  • The dropdown withholds it from anyone who does not own the item. A row already set to manage keeps the option, so a delegate opening the dialog cannot silently downgrade the owner's own grant, and a mixed selection follows its strictest item.
  • The server says why: cannot_delegate_manage, "Only the owner can grant edit & share access" — told only to someone who can already share the item, so a stranger still gets the ACL's own safe error that does not admit the node exists. There is a test for that leak.

After:

delegate grants manage -> error: [cannot_delegate_manage] Only the owner can grant edit & share access

If the intent was that a delegate should be able to pass on edit & share, this is the wrong fix — that is a permission-model change, and worth deciding before anyone writes it. This PR keeps the existing policy and fixes the affordance.

PUT-1599 — "Shared with X" when nothing changed

Sharing the same file with the same person twice answered exactly as a first share. Verified the wire was the problem, not the dialog: both calls returned byte-identical payloads.

The service already knew — it computes isNew to decide whether to notify the recipient — but the flag stopped at the controller. It now travels on share results; a listing describes standing access, so it stays out of getShares().

Which of the three things happened is settled client-side, because the mode each recipient holds is already on screen:

Outcome Message
created access Shared with X
raised or lowered the level Updated access for X
changed nothing X already has this access
recipient has no account yet Invited X …

That keeps the previous mode off the wire. The lookup matches on the resolved username from the response rather than what was typed, so an email belonging to a known account still finds their row.

Live, after the change:

first share  mode=read  is_new=true
same again   mode=read  is_new=false
mode change  mode=write is_new=false
listing carries is_new? false

Verification

Both fixes were driven through the real dialog in a browser, not just asserted at the API. The GUI loads puter.js from the CDN, which predates isNew, so every page in the run has that URL routed to the local dist/puter.dev.js — otherwise the dialogs are exercising an SDK that cannot carry what this PR added. The run asserts that before doing anything else, and aborts if the page ends up on the CDN build.

Owner and delegate are separate accounts in separate browser contexts; the share modal is opened the way a user opens it, by right-clicking a row and choosing "Share…".

# Check Result
1 Owner is offered all three levels ["read","write","manage"]
2 First share reads as a share "Shared with v6uq49v"
3 Same access again says nothing changed "v6uq49v already has this access"
4 A different level reads as an update "Updated access for v6uq49v"
5 A delegate is not offered "can edit & share" ["read","write"]
6 A delegate can still grant what they may "Shared with vsdw6be"

6/6 checks passed. Each check is shown below, in the order above.

1 — the owner is offered all three levels. "Can edit & share" is theirs to give.

put-1586-owner-modes

2 — a first share. "Shared with v6uq49v", as before.

put-1599-1-first-share

3 — the same access again. "v6uq49v already has this access" — this is PUT-1599; it used to repeat the message above.

put-1599-2-already-shared

4 — a different level. "Updated access for v6uq49v".

put-1599-3-access-updated

5 — the delegate's dialog. The picker offers "Can view" and "Can edit" only; "Can edit & share" is gone. This is PUT-1586. Their own row below still reads "Can edit & share", which is deliberate — an existing grant is never silently downgraded by opening the dialog.

put-1586-delegate-modes

6 — the delegate sharing what they may. "Shared with vsdw6be" at view level.

put-1586-delegate-shared

Check 5 runs as the owner's own session on an item someone else owns and has given them manage — the same branch a delegate takes, without a second GUI login. Its access list shows the delegate's own row still reading "Can edit & share": a row already set to it keeps the option, so opening the dialog cannot silently downgrade the owner's grant.

Alongside that:

  • Backend: a wire-level HTTP test that is_new flips across repeat shares and that listings omit it; the delegate test pins cannot_delegate_manage, plus a test that a stranger asking for manage does not get that code.
  • GUI: 7 unit cases for share_outcome, 2 for the manage filter. 360 passing.
  • SDK: isNew asserted in the sharing suite; check:puterjs:types clean.
  • API level, against a running server: read and write still succeed for a delegate, manage answers cannot_delegate_manage; is_new is true then false across repeat shares and absent from listings.

A recipient given "can edit & share" could not pass that level on: the
dialog offered it, the server refused it, and the refusal was a bare
403 Forbidden that reads as a bug. Handing out manage needs authority
over manage, which only the owner has — the refusal is right, the dead
end and the silence were not.

The dropdown now withholds it from anyone who does not own the item; a
row already set to it keeps it, so opening the dialog cannot downgrade
the owner's own grant, and a mixed selection follows its strictest item.

The server says why, and only to someone who can already share the item
— a stranger still gets the ACL's own safe error, which does not admit
the node exists.

Verified against a running server: a delegate grants read and write as
before, and manage now answers cannot_delegate_manage with a sentence
naming the owner as the one who can.
Sharing a file with someone who already had it answered "Shared with
X", the same as a first share, so the dialog claimed to have done
something it had not.

The service already knew — it computes isNew to decide whether to
notify the recipient — but the flag stopped at the controller. It now
travels on share results only; a listing describes standing access and
says nothing about it.

Which of the three things happened is settled client-side, since the
mode each recipient holds is already on screen: created access, raised
or lowered it, or changed nothing. That keeps the previous mode off the
wire, and an older backend that omits isNew still reads as a share,
which is what these dialogs said before.
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for puter.js SDK

Status Category Percentage Covered / Total
🔵 Lines 62.12%
⬆️ +0.04%
3942 / 6345
🔵 Statements 60.94%
⬆️ +0.03%
4154 / 6816
🔵 Functions 62.31%
🟰 ±0%
759 / 1218
🔵 Branches 54.92%
⬆️ +0.02%
2552 / 4646
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/puter-js/src/modules/FileSystem/operations/shareUtil.js 80%
🟰 ±0%
68.85%
⬆️ +1.06%
88.88%
🟰 ±0%
85.18%
🟰 ±0%
45-46, 99, 102-103
Generated in workflow #196 for commit 4b3d111 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 93.66%
🟰 ±0%
24573 / 26235
🔵 Statements 91.94%
🟰 ±0%
26545 / 28869
🔵 Functions 89.88%
🟰 ±0%
4292 / 4775
🔵 Branches 80.58%
⬆️ +0.01%
17936 / 22258
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/backend/controllers/share/clientShare.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
src/backend/services/share/ShareService.ts 92.54%
⬆️ +0.06%
81.3%
⬆️ +0.23%
95.12%
🟰 ±0%
95.39%
⬆️ +0.04%
77, 88, 100-104, 109, 116-120, 133, 183, 184, 267-269, 301-306, 355, 360, 370, 459, 482, 518, 544, 608, 750, 770, 835, 885, 893-894, 904-905, 920-921, 930, 943-944, 952-956, 1013-1014, 1046-1048, 1069-1071, 1100, 1247-1248
Generated in workflow #839 for commit 4b3d111 by the Vitest Coverage Report Action

@jfcastro92
jfcastro92 requested a review from Salazareo August 26, 2026 16:30
@jfcastro92
jfcastro92 merged commit 3d28b2b into juancastro/put-1590-sharing-shared-files-are-not-different-than-regular-files Aug 26, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants