[SudoThijn]

Summary

Closes #1264. When team offboarding hands a secret to a successor, its link shares stayed with the leaving user: LinkShareService ties list and revoke rights to a link's created_by, so the leaving user could still manage the links and the successor couldn't. The links now move with the secret, and Keepiq keeps who made them.

Transfer

• SecretOwnerTransferService::transfer(secretId, fromOwnerId, newOwnerId) reads the owner inside its transaction. When that owner is still fromOwnerId, it moves the links fromOwnerId holds on the secret to the new owner (LinkShareMapper::reassignCreator), in the same transaction as the secret and its partner shares. It returns the number of links moved.
• Links a delegate created on the secret stay with that delegate. On account deletion a secret delegated twice is handed over twice; the second hand-over moves no links, so the first delegate keeps theirs.
• A missing secret still has its partner shares moved; only the link step is skipped, since deleting a secret deletes its links.

The maker of a link

created_by is who manages a link. A new nullable original_created_by column keeps who made it once it is handed over: reassignCreator and reassignCreatorById set it to COALESCE(original_created_by, created_by) before they overwrite created_by, so a chain of transfers keeps the first maker. NULL means the holder made the link.

• The compromise revoke (MigrationService::revokeKeyMaterialOfOwner, used by the owner's recovery and the admin force-revoke) cuts the links a user made through LinkShareService::deleteMadeBy: their own links and the ones handed to someone else, not the links handed to them.
• The data export (GdprService::collectLinkShares) lists the links a user holds and, flagged handedOver, the links they made that were handed to someone else.
• Account deletion replaces the deleted user as maker with keepiq:deleted-account, which Nextcloud does not accept as a user ID (: is not allowed), so the holder's compromise revoke still leaves those links. The export and the compromise revoke also guard against a user backend that skips that check and hands out the marker as a uid.

The upgrade adds the column (Version001014Date20261019120000), with a version bump.

Account deletion

AccountDeletionService::deleteAllFor deletes the account's link shares and replaces it as maker before it hands delegated secrets on. Without that, the transfer would move the account's links to the delegate and they'd survive the deletion, against the GDPR cascade. This also keeps an interrupted run safe to repeat.

Repair

ReownTransferredLinkShares (post-migration) hands links left behind by an earlier offboarding to their secret's current owner, and records the maker. A permanent delegation is the record of the transfer: a link matches when its creator is that delegation's original owner for the same secret, it was created no later than the delegation, and the creator no longer owns the secret. Links are moved one at a time by id, so a former owner's later link as a delegate on the same secret stays theirs. It handles chained transfers, is idempotent, logs each moved link id, and leaves delegate links, temporary delegations, suite-revocation delegations, application-owned secrets and dev seeds alone.

Audit

The TEAM_FOLDER_OFFBOARDED audit event carries linkSharesTransferredCount. The original creator of each link stays in its LINK_SHARE_CREATED event.

Docs

• openspec/specs/team-folder-sharing/spec.md: the offboarding requirement covers link shares and their maker, with a new scenario, and the offboarding audit event carries the link count.
• openspec/specs/encryption-suites/spec.md: the compromise revoke cuts the links the user made, also those handed to a successor, and not the links handed to them.
• openspec/specs/gdpr-compliance/spec.md and docs/gdpr.md: deletion removes the link shares the user holds, also on secrets that pass to a delegate, and replaces the user as maker of links others hold; the export lists handed-over links.
• CHANGELOG [Unreleased] Fixed: a Link shares stay with the previous owner after team offboarding #1264 entry.

Follow-ups (not in this PR)

• ShareRevocationService::revokeShare deletes a recipient's copy but not the links made from it, so links the leaver made from revoked team copies keep working and only the leaver can manage them.
• On account deletion, a secret with two delegations is handed over twice and the last delegate becomes its owner.
• SecretService::delete leaves a secret's partner shares behind.

Test plan

• composer test:unit in the stable35 container (MariaDB): 2342 tests, no failures, 3 unrelated skips in NextcloudFloorMatrixTest. The DB-backed LinkShareOwnerTransferTest runs, not skipped.
• composer phpcs, phpstan and phpmd pass. Psalm crashes on an environment error before it reaches app code, as on development.
• The new original_created_by update gives the same result on MariaDB, PostgreSQL and SQLite.
• Mutation checks, each failing a test: reverting the deletion reorder; dropping the link move; using the wrong from-user; moving it outside the transaction; moving links when the owner is no longer fromOwnerId; the compromise revoke cutting by created_by only, or without either half of its condition; not recording the maker, dropping the COALESCE or swapping the SET order (red on MariaDB; PostgreSQL, which CI's PHPUnit leg uses, and SQLite read the old created_by either way, so only a MySQL-family run catches it); writing NULL instead of the deleted-account marker, or skipping it on account deletion; dropping either guard that keeps a uid equal to the marker from reaching the marker's links; the export finding only held links; dropping the audit count or the repair's log line; and removing the repair's is_permanent, lte or owner_type filter or its original_owner_id join.
• Not covered by a single mutation: the repair's owner ≠ creator filter is an equivalent mutant (reassignCreatorById skips a link its holder already owns), and the dev seeds are protected by both is_permanent and that filter.
• Independent review and rereviews.

🤖 Generated with Claude Code

[github-actions]

Quality Report — ConductionNL/keepiq @ 54374e4

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 114/114
npm			✅	✅ 660/660
app:check-code	⏭️
info.xml	✅
REUSE	✅
lockfile sync		✅
PHPUnit					✅ 2328/2328 · 3 skipped
Newman					✅ 177/177 · 20 skipped
Playwright					⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates					✅

---

Quality workflow — 2026-10-19 09:05 UTC

Download the full PDF report from the workflow artifacts.

[rjzondervan]

Verdict: REQUEST_CHANGES (Strict)

The offboarding path does what #1264 asks: the leaver's links on a handed-over secret move to the successor in the same transaction, and the repair catches up past offboardings without touching delegate links, suite-revocation delegations or application-owned secrets. Two blockers come from rewriting created_by itself: on account deletion a second delegation moves the first delegate's own links, and moved links fall out of the leaver's compromise revoke.

🔴 Blockers (2)

• A second delegation on account deletion moves the first delegate's own links to the second — lib/Service/SecretOwnerTransferService.php:91. transfer() re-reads the owner, so the second hand-over of a twice-delegated secret runs reassignCreator(secret, D1, D2); it contradicts "link shares a delegate created stay the delegate's" (SecretOwnerTransferService.php:10, CHANGELOG.md:24).
• Overwriting created_by takes the leaver's links out of the leaver's compromise revoke — lib/Db/LinkShareMapper.php:162. revokeKeyMaterialOfOwner cuts by created_by because the snapshot carries the creator's key (encryption-suites/spec.md:209, :771-773); after a move, and through the repair for past offboardings, the leaver's revoke misses those links and the successor's cuts them. The PR body's follow-up 2 places this outside the PR, but this diff causes it.

🟡 Concerns (1)

• No test covers the repair's is_permanent filter — lib/Repair/ReownTransferredLinkShares.php:125. Removing it changes no assertion, so "loosening the repair's filters each fails a test" does not hold for it; the dev-seed exclusion has no test either.

🟢 Minor (1)

• The link hand-over is not in the audit log — lib/Repair/ReownTransferredLinkShares.php:101. Neither the runtime move nor the repair records which links changed manager; the original creator stays recoverable from LINK_SHARE_CREATED.

✅ Verified clean

• The previous owner is read inside the transaction; a missing secret skips only the link step and its partner shares still move; a failed link write rolls the whole transfer back.
• Account deletion deletes the account's links before handing delegated secrets on, so an interrupted run is safe to repeat; its test goes red with the reorder reverted.
• All 7 new SecretOwnerTransferService( call sites are updated; production code is DI-wired.
• The repair: registered post-migration only; its joins and DATETIME/PARAM_BOOL comparisons are portable across MySQL, PostgreSQL and SQLite; it moves links one at a time by id, resolves chained transfers to the current owner, is idempotent, and leaves delegate links, suite-revocation delegations and application-owned secrets alone (each with a test).
• Team offboarding enumerates only the leaver's user-owned secrets, each with a fresh permanent delegation dated now, which the repair's lte relies on.
• The version bump has its CHANGELOG entry under [Unreleased].

Persistence audit

15 vectors. Three come from this PR: the two blockers above and the missing audit record. The others are pre-existing and untouched here: links made from revoked team copies (follow-up 1), the leaver's temporary delegations surviving the transfer, the leaver staying a delegate on other people's secrets, partner shares made from deleted copies (follow-up 4), no re-keying on transfer, and links that keep working after offboarding by design.

Connected issues

Issue	State	This review
#1264	open	the offboarding move is done; the account-deletion path (f1) and the compromise revoke (f2) are not yet right; closes on merge

CI: every check green on 80771dd, including the required ones; E2E (Playwright) was skipped.
Not run: PHPUnit locally; the PR's mutation claims were checked by reading the tests, which is how f3 was found.

🤖 Generated with Claude Code

[github-actions]

Quality Report — ConductionNL/keepiq @ 3366075

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 114/114
npm			✅	✅ 660/660
app:check-code	⏭️
info.xml	✅
REUSE	✅
lockfile sync		✅
PHPUnit					✅ 2339/2339 · 3 skipped
Newman					✅ 177/177 · 20 skipped
Playwright					⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates					✅

---

Quality workflow — 2026-10-19 12:33 UTC

Download the full PDF report from the workflow artifacts.

[rjzondervan]

Verdict: APPROVE (Strict, round 2)

Nothing left to block: all 4 round-1 findings are fixed in a16b7497, and the 3 new findings are 🟢. Before a merge: one approval (this one), and 3 open threads (this round's 🟢s).

Round	Found	Status
1 (80771dd)	🔴 2 · 🟡 1 · 🟢 1	all resolved in a16b7497
2 (a16b749) — this review	🟢 3	open

Finding	Was	Now	Where
🔴 A second delegation on account deletion moves the first delegate's own links to the second	🔴 round 1	resolved in a16b7497	thread
🔴 Overwriting created_by takes the leaver's links out of the leaver's compromise revoke	🔴 round 1	resolved in a16b7497	thread
🟡 No test covers the repair's is_permanent filter	🟡 round 1	resolved in a16b7497	thread
🟢 The link hand-over is not in the audit log	🟢 round 1	resolved in a16b7497	thread
🟢 The compromise-recovery guide and the containment scenario still say every link share is revoked	new	new — 🟢	comment
🟢 The deletion report leaves out the marker replacement	new	new — 🟢	comment
🟢 No test reaches the repair's new zero-rows branch	new	new — 🟢	comment

Details of this round

• Scope: one commit since 80771dd: a16b7497 (30 files, +678/−69), analysed at Strict depth with a delta-scoped persistence audit.
• Verified clean: the migration (hasTable/hasColumn/hasIndex guards, nullable original_created_by of the same length as created_by, a named index the compromise cut uses, no data update); original_created_by is set before created_by in the same UPDATE, as MySQL's left-to-right evaluation needs; every transfer( caller and all 7 new SecretOwnerTransferService( sites pass fromOwnerId; the deleted-account marker is not a valid uid for core or LDAP, and a colliding uid reaches none of its links; account deletion deletes held links, writes the marker, then transfers, and a re-run is idempotent; the GDPR export's handedOver rows carry no token or snapshot.
• Persistence audit: the round-1 vectors are covered (the leaver's compromise revoke, the second delegation) or partly so (the audit trail has counts and info-level ids). Links moved by the round-1 repair before the column existed have no original_created_by; only installs that ran the round-1 branch have such rows, so not a finding.
• CI: green on a16b749, including quality / Hydra Gates.

Before the next round: fix rules on (fix-induced 2 + missed-sibling 0 ≥ new 0): this round's findings came from the fix, so the next fixes go through the class sweep (every copy of the "revoked" sentence: git grep -n "Revoked\.\|link shares MUST be revoked" -- docs openspec).

🤖 Generated with Claude Code

[github-actions]

Quality Report — ConductionNL/keepiq @ e3340f0

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 114/114
npm			✅	✅ 660/660
app:check-code	⏭️
info.xml	✅
REUSE	✅
lockfile sync		✅
PHPUnit					✅ 2340/2340 · 3 skipped
Newman					✅ 177/177 · 20 skipped
Playwright					⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates					✅

---

Quality workflow — 2026-10-20 09:36 UTC

Download the full PDF report from the workflow artifacts.