Release: merge beta into main - #828
Merged
Merged
Conversation
"Vault unlocked. Opening your vault…" was hand-inserted into en.json mid-file — not appended, so it did not come from `npm run test:l10n:write` — and it appears nowhere in src/, lib/ or templates/. Nothing was going to catch it either: check-l10n.js only asserts used -> present, never present -> used. It was not harmless bookkeeping. With ENFORCED defaulting to REQUIRED the key is permanently enforced across 36 locales x 2 files, so it would have had to be carried and kept non-empty forever for a message that is never displayed. The string belongs to the unlock-animation work, which already carries it in all 37 catalogues alongside the LockScreen call site that renders it, so it lands with the change that uses it rather than here. en.json is back to the 1125 keys it had at the merge base, which means this branch now adds no English source string at all.
Moving sr from Croatian Latin to Serbian Cyrillic rewrote only the values
that were still identical to hr, which left untouched the strings that were
already correct Serbian *Latin*. The catalogue came out at 1,070 Cyrillic
and 51 Latin: `Dashboard` rendered as `Kontrolna tabla` beside `Трезор`,
`Тајне` and `Извештаји`, with `Try again`, `Refresh`, `Admin settings`,
`Create vault`, `Filter by type` and `More actions` equally prominent. The
tour copy embedded Latin labels inside Cyrillic prose. Before the move sr
was at least uniformly Latin; after it, internally mixed.
Serbian is digraphic, so either script alone is fine — mixing them within
one catalogue is not. All 52 remaining Latin values are transliterated,
digraph-aware for lj/nj/dž, leaving only URL, BSN, CVV and
`Keepiq {version}`.
Four of them name a navigation item, and there a letter-for-letter
transliteration would have named something the menu does not show: the
Latin block said "Funkcije i plan razvoja" while sr's own label for
Features & roadmap is "Могућности и планови", and it left Flows in English
where sr calls it "Токови". Those four use the real labels instead.
uk was seeded from ru and only partly translated, and it was described as carrying Russian deliberately. Its own numbers say otherwise: 355 of the 581 seeded keys (61.1%) were still identical to ru, against 13 of the 545 keys added later (2.4%). That step change is the same signature that identified ca, lb, rm and the rest as contaminated — a uniformly high figure, as bs shows, is what a genuinely close language pair looks like. 121 values carried ы/э/ъ/ё, letters the Ukrainian alphabet does not have, 194 occurrences in total, and core UI was affected: Save read Сохранить, Cancel Отмена, Settings Настройки, Vault Хранилище, Secrets Секреты, Loading… Загрузка…. The keys this branch adds are proper Ukrainian, so the file was half correct and half donor text. 319 values are now Ukrainian, following the vocabulary the already-correct part of the file established: сховище, секрет, тека, спільний доступ, застосунок. The 49 that stay identical to ru are words the two languages genuinely share — Назад, Причина, Система, Пароль — plus acronyms and brand names, which is the residual overlap the cleaned locales also show (49 for ca, 68 for lb, 74 for sv). No value contains a non-Ukrainian letter any more.
be was seeded from ru exactly as uk was, and shows the same split: 349 of the 581 seeded keys (60.1%) still identical to ru, against 6 of the 545 keys added later (1.1%). 244 values carried и/щ/ъ — letters absent from the Belarusian alphabet — across 550 occurrences, so more than a fifth of the catalogue was demonstrably not Belarusian. 323 values are now Belarusian, following the vocabulary the clean part of the file established: сховішча, сакрэт, папка, супольны доступ, праграма. Two strings this branch itself added also said ссылка where be uses пасылка; both are corrected while the file is open. The 34 left identical to ru are genuinely shared words and acronyms, in line with the other cleaned locales, and no value contains a non-Belarusian letter any more.
The comment claimed `precedence = "closest"` completes the root LICENSE's half header (copyright, no SPDX tag). It does not: reuse-tool's _IGNORE_FILE_PATTERNS matches `^LICEN[CS]E([-\.].*)?$` before any REUSE.toml annotation is consulted, so LICENSE is never covered and was never counted as non-compliant. `reuse spdx` on this branch emits 1467 FileName: entries and ./LICENSE is not among them. The tests/e2e/visual/_visual-helpers.ts half of the example is correct and stays. Also drops the "of 1470 tracked files" denominator, which was neither the tracked count before this branch (1471) nor reuse's own (1466).
The License section promised EUPL-1.2 but said nothing about REUSE, so a contributor had no way to know that new files need no SPDX header, that their own header wins over the repo-wide blanket, or that vendoring a third-party file also needs that licence's text in LICENSES/. With the REUSE job non-blocking, this note is the control.
feat(l10n): complete all 36 locales and hard-enforce parity
The shared quality.yml defaults `reuse-blocking` to false because most fleet apps ship no REUSE.toml or LICENSES/ directory, and its failure hint says to flip it once an app ships both. This branch ships both and lints at 1467/1467, 0 missing, 0 unused, compliant with REUSE 3.3. The blanket `path = "**"` annotation makes every new file compliant on arrival, so blocking cannot be tripped by ordinary work. It leaves one regression mode: vendoring a file whose own SPDX header names a licence with no text in LICENSES/. CONTRIBUTING.md's REUSE note said the gate "reports" that case; it now fails the build, so the sentence says so.
Drops the narration about the editing itself — what the previous comment said, which repo the form came from — and keeps every measured fact that constrains a future edit. Removes the concurrency preamble outright rather than shortening it: it described suffixing only the default-branch push, which the block directly below it reverses, and asserted that feature-branch dedup was untouched (`quality-feature/x` for both events). That has not held since `push:` narrowed to main and development, where feature branches get no push run at all. The facts it carried — the push-only jobs, the standing sync PR whose head_ref IS the branch name — are stated in the two blocks that remain. jobs.quality.if had no comment; it now names the sync PR it skips. The Licensing note gets the lint result and the reason blocking is safe, which the four-line version had cut too far. No setting changes: verified by parsing both versions and walking them key by key.
Comments across the tree pinned behaviour to the stage of the restyle that produced it — "restyle Stage 9", "the Stage-5 terminology split", "Restyle Stage 8: the detail is a right sidebar". The stage numbers were working structure for that effort and index nothing a reader can look up; the behaviour they describe is the part that matters and stays. 70 references removed across lib/, src/ and tests/. Where the number was load-bearing in the sentence it is replaced by what it referred to rather than deleted: the Stage-5 terminology split is the vault/folder split, the Stage-5 toolbar actions are the page's own toolbar actions. One test title loses the number with it. The fuzzy-search Stage 1 / Stage 2 in SecretService and its archived openspec proposal are untouched — those name the two phases of the search algorithm, not the restyle. A dangling reference to RESTYLE-PLAN.md, which is not in the repo, goes with the comment that carried it.
chore(license): declare REUSE/SPDX licensing for the whole tree
…trees npm audit reported 9 findings (6 low, 3 moderate). Neither cluster had an upstream fix, so both are resolved by removing the vulnerable package from the tree rather than by bumping a version. 6 low — the elliptic chain -------------------------- elliptic has no patched release (GHSA-848j-6mx2-7j84 covers <= 6.6.1 with first_patched_version: null), so no version bump can clear it. Its only entry point was @nextcloud/webpack-vue-config, which peer-pins [email protected] → crypto-browserify → browserify-sign / create-ecdh → elliptic. Bumping the polyfill plugin does not help either: [email protected] still depends on crypto-browserify. Drop @nextcloud/webpack-vue-config and inline the parts of its base config this app actually inherited. That is a small surface: module.rules and resolve were already replaced wholesale here, and the package's babel-loader and ts-loader rules were dead code (no .babelrc, no tsconfig.json). Its devServer block goes too — there is no `webpack serve` script. NodePolyfillPlugin is replaced by an explicit ProvidePlugin for Buffer and process (exactly what its alias filter admitted under additionalAliases: ['process']) plus a resolve.fallback for the only node builtins the graph reaches, measured against a real build: stream, path, buffer, events, process, string_decoder, and fs: false. Any builtin not listed now fails the build with webpack's own "add a fallback" error instead of being polyfilled silently. The five polyfill packages become declared dependencies; they were already resolved at these versions, so the install adds nothing. minimizer-webpack-plugin becomes a declared devDependency — it was only reachable as a peer of the package being removed. 3 moderate — dompurify ---------------------- Override dompurify to the root range so the tree holds one deduped copy, removing the nested @toast-ui/editor/node_modules/[email protected]. The nested-override form ("@toast-ui/editor": { "dompurify": ... }) does not apply, so this uses the top-level form. This clears the audit but does not change the shipped bundle: @toast-ui/editor inlines DOMPurify 2.3.3 into its own dist, so the package copy npm was flagging was never imported by anything. That code is still bundled, and no change in this repo can reach it — the fix belongs in @conduction/nextcloud-vue's CnMarkdownEditor. Result: npm audit reports 0 vulnerabilities, also with --omit=dev. The lockfile goes from 1399 to 1118 packages (281 removed, 0 added), which also drops @babel/core, babel-loader, ts-loader and webpack-dev-server — peers of the removed package that nothing here used. Verified: full dev build compiles and emits an identical set of 225 asset names; 838 vitest tests pass; eslint, stylelint, manifest and l10n parity green; hydra gates 78/79 applicable green (gate-4 composer-audit does not run locally), matching the pre-change baseline.
Opening any vault flooded the console with one Vue warning per crumb per
render:
[Vue warn]: Invalid prop: type check failed for prop "name".
Expected String with value "undefined", got Undefined
NcBreadcrumb declares `name` as a required String, and the trail's first
crumb is icon-only -- `{ icon: 'Home', to: ... }`, the shape CnBreadcrumbs'
own docs advertise for a Home crumb -- so `crumb.label` reached it as
undefined. The trail is rendered only inside a folder, which is why the
root page was quiet and every vault was not.
The crumb now carries `label: t('keepiq', 'Vaults')`. Nothing renders it:
NcBreadcrumb prints `name` only for crumbs without an icon slot, so the
crumb looks exactly as before. The word matches the nav item pointing at
the same route, and the key is already translated across l10n/, so this
adds no string.
CnBreadcrumbs stops forwarding undefined on its own side too, but keepiq
builds the published @conduction/nextcloud-vue dist, so that fix reaches
this app only on the next release -- the label is what silences it now.
Not addressed here: the Home crumb's button has no accessible name at all.
NcBreadcrumb sets aria-label only when its `icon` prop is used, not the
`#icon` slot CnBreadcrumbs renders through.
With the page title hidden in keepiq, the actions bar is the page's TOP element: `.cn-index-page` gives up its remaining 4px of top padding, and the bar's top corners go square so the bar meets the page edge as a seam instead of showing the page background in two notches. The bottom corners keep the container-large radius, so the shorthand is split into logical longhands rather than losing the token and its fallbacks for older server generations.
The "Applications awaiting approval" widget listed the pending registrations
and then left an admin with nowhere to go: its configured "View all" footer
is dead. `limit` never reaches the widget on @conduction/nextcloud-vue
2.36.3 -- the object-table host adapter destructures it out to fold it into
`source`, and an endpointSource table has no `source` -- so every row
renders, the footer's total-greater-than-shown condition can never hold, and
`viewAllRoute` is unreachable no matter how many applications are waiting.
`rowRoute: "ApplicationRegister"` gives the card its navigation back: any
row opens /applications, the page where the queue is handled. A route NAME,
so the widget router-pushes it -- the vault master key is memory-only, and a
full page load would re-lock the vault and land on /lock, which is the trap
this manifest's tile note already documents for linkType "app".
Deliberately NOT ApplicationDetail (/applications/:id, which does exist):
the queue's job is to get an admin to the page that handles the whole queue,
not to one application. The cost is one dev-only console line per click --
the widget's rowRoute always pushes `params: {id}` and vue-router 4.6.4
discards a param this path has no segment for (measured: it resolves to
/applications and warns "Discarded invalid param(s) id"). Production builds
strip the warning.
The widget's `_note` is corrected with it. It claimed "No rowRoute:
applications have no detail page", and ApplicationDetail has existed for
some time; the note now records why the row still goes to the index, and
what makes the View-all footer dead rather than merely unconfigured.
The upstream adapter fix is committed in nextcloud-vue, so the footer starts
appearing at a fourth pending application once keepiq bumps past 2.36.3.
Until then rowRoute carries this on its own.
terser-webpack-plugin was declared in `dependencies` but referenced nowhere in the repo. A scan of every package in the lockfile confirms the root manifest was its only dependent: webpack 5.110 minifies through `minimizer-webpack-plugin`, which this config already uses explicitly, and which carries `terser` as a direct dependency of its own. Because it sat in `dependencies` rather than `devDependencies`, it and ~50 transitives were inside the `--omit=dev` audit scope. The lockfile diff is the plugin's own entry plus those packages flipping to devOptional. `npm audit` remains 0 findings both with and without `--omit=dev`.
The output literal inlined `publicPath: '/apps/<app>/js/'` from the base config
and then overwrote it 90 lines later with `publicPath: 'auto'`, so the inlined
value never took effect. That was reasonable while the base config was an opaque
package; now that both live in one file it just reads as a contradiction.
Set `publicPath: 'auto'` directly in the literal, carrying the explanation of
why the `/apps/` path 401s under `/custom_apps/`, and drop the override block.
Also drops three `|| {}` guards that defended against objects this file now
defines itself, and corrects the header's claim that only the listed fields were
inherited — `output.publicPath` was inherited and is deliberately not kept.
Behaviour is unchanged: requiring the config before and after under
NODE_ENV=production yields an identical resolved object.
The build row named `@nextcloud/webpack-vue-config`, which is no longer installed — a reader following it would look for a package that is not there. The frontend row said Vue 2.7 while package.json declares vue ^3.5.40 and the webpack config is written around Vue 3 / vue-loader@17.
Reported from testing, while clearing the development seed: deleting several
secrets ran fine and then ended on a screen that was part result, part
command -- with a live "Delete 0 secrets" button on it.
Everything in the dialog counted off the SELECTION, and the host reloads the
list when the run completes (SecretList.onBulkDone), after which the
reconciled selection is empty. So a finished run left a warning about
deleting 0 secrets, a destructive button offering to delete 0 secrets, and
the per-item report, all at once.
The dialog now has two phases and the phase decides the chrome. While it
asks, nothing changes. Once the run has finished the warning and the typed
confirmation are gone, the destructive button is REMOVED rather than
disabled, Close becomes the primary action, and the title turns into an
outcome: "Deleted {ok} of {total} secrets", counted off the report so it
stays true when the selection behind it is empty and when part of the run
failed or was cancelled.
The phase is an instance flag, not a read of the store. The report outlives
the dialog -- it is cleared with the selection, not on close -- so deriving
"finished" from it would have opened a fresh dialog straight into a result
state. The same flag now gates BulkRunPanel, which fixes a second, quieter
version of the same confusion: reopening the dialog showed the PREVIOUS
run's report table before anything had been asked for.
A failed run keeps its report and its Retry button; only the affordances
that would act on the emptied selection go away.
l10n: one new key, translated into all 36 required locales rather than left
to fall back to English, and the .js catalogues regenerated from the .json
(npm run l10n:build). Each translation follows the phrasing that locale
already uses -- the noun from its own "Delete {count} secrets", the "X of Y"
construction from its "Page {page} of {total}" / "Used {used} of {limit}".
The three sibling bulk dialogs (move / share / team folder) share the
pattern and are fixed in the commit after this one.
The same defect the delete dialog was just fixed for, in the three dialogs
beside it. Everything counted off the SELECTION, and the host reloads the
list when a run completes, which empties the reconciled selection -- so a
finished run left a title counting 0 secrets, a destination picker, and a
live primary button.
Quieter than delete only because their run buttons carry static labels
("Move", "Share", "Add to team folder") rather than a count. Worse in one
respect: pressing the button again ran the runner over an empty id list,
which RESET the report to empty -- the record of what had just happened was
one stray click away from being wiped.
Each now has the two phases delete has. While it asks, nothing changes. Once
the run has finished the input is gone, the run button is REMOVED rather than
disabled, Close becomes primary, and the title turns into an outcome counted
off the report: "Moved/Shared {ok} of {total} secrets", "Added {ok} of
{total} secrets to the team folder". The same instance flag gates
BulkRunPanel, so reopening a dialog no longer shows the previous run's table
before anything has been asked for.
Two details that are not copy-paste from delete:
• The team-folder run is two steps -- the chunked move, then the
membership fan-out -- so its switch waits for `fanOut.running` as well.
Reporting while members were still receiving copies would have been a
new version of the same lie.
• Share resolves the recipient's certificate BEFORE the runner starts and
returns early when the recipient has no active suite. The flag is set
only after that resolves: such a dialog never ran, so it must keep
asking with the reason on screen.
Tests: tests/dialogs/BulkPhaseDialogs.spec.js runs the asking phase, the
switch to reporting, and the fresh-open case over all three dialogs
(describe.each), plus the refused-recipient path for share. Verified they
fail without the change.
l10n: three new keys, translated into all 36 required locales and the .js
catalogues regenerated, each following the verb and "X of Y" construction
that locale already uses.
…trees fix(deps): clear all npm audit findings by pruning the vulnerable subtrees
`locator('.lock-screen').getByText(/Wrong master password|decryption
failed/i)` matched two elements and failed under strict mode on a page
that was behaving correctly:
1) <p role="alert" class="lock-screen__sr-live">
2) <p class="input-field__helper-text-message">
Both are meant to be there. The helper text under the field is what a
sighted reader sees; the visually-hidden role="alert" is what a screen
reader announces, and it was added deliberately so a rejected credential
interrupts rather than going unspoken.
So both are asserted, rather than the match being narrowed to whichever
one is convenient. Narrowing would let the other be removed with no test
noticing, and for the live region that means losing the announcement
silently — the failure mode the region exists to prevent.
Co-authored-by: Conduction Release Bot <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
hydra-gates v1.10.0 -> v1.16.0 nc-vue - -> 2.36.3 Lock-only: both packages are already declared with caret ranges that permit these versions, so nothing about what this app ACCEPTS changes - only what it currently resolves to. Opened by the weekly fleet shared-dependency bump, because a lock nobody re-resolves is a pin nobody chose. Merging is gated by this repository's own suite, deliberately: taking hydra-gates v1.8.1 added patchObject() to a published interface, which is a load-time fatal for any concrete double that implements it without the method. CI is the only thing that can tell a safe bump from that. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
#642 fixed this strict-mode violation in `tests/e2e/workflows/vault-unlock.spec.ts` and left the identical locator in `tests/e2e/spec-coverage/lock-screen.spec.ts` untouched, so keepiq's development E2E stayed red on the same defect: strict mode violation: locator('.lock-screen') .getByText(/Wrong master password|decryption failed/i) resolved to 2 elements: 1) <p role="alert" class="lock-screen__sr-live"> 2) <p class="input-field__helper-text-message"> I reported keepiq as fixed after #642 merged. It was not: I had fixed the spec that failed rather than the LOCATOR that was wrong, and the second copy failed the very next run on a tree containing the fix. Same treatment as #642, so the two files now agree. Each surface is asserted on its own: the helper text is what a sighted reader sees, the visually-hidden role="alert" is what a screen reader announces, and it was added deliberately so a rejected credential interrupts rather than going unspoken. Narrowing to either one alone would let the other be removed with no test noticing, which for the live region means losing the announcement silently. The two remaining uses in `tests/e2e/workflows/_workflow-helpers.ts` are `.count()` and `.first().textContent()`. Neither is strict-mode sensitive, so they are correct as they stand and are left alone. prettier clean. Co-authored-by: Conduction Release Bot <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Roughly half of the config literal in webpack.config.js was inlined verbatim from @nextcloud/webpack-vue-config (AGPL-3.0-or-later, © Nextcloud GmbH) when that package was dropped to clear an elliptic advisory with no patched release. The file carried no header of its own, so REUSE.toml's repo-wide blanket declared it Conduction B.V. / EUPL-1.2 — a licence and a copyright that are not ours to assert over that code. Give the file a dual header. `precedence = "closest"` means it wins over the blanket, the same arrangement .editorconfig already uses for its own Nextcloud origin. LICENSES/AGPL-3.0-or-later.txt is already present. REUSE.toml's comment enumerated .editorconfig as the only file whose own header wins; it now names both and says why this one differs. No behaviour change: the resolved webpack config is byte-identical.
…header Review of #647 found that the copyright glyph in the header's prose is read by REUSE as a copyright statement anywhere in a file, not only inside an SPDX tag, so it reported a third holder whose name was the remainder of the sentence. Drop the glyph — the tag on line 1 already carries the attribution — and say so in the header without naming any of the three marker forms literally. Also from that review: - The header claimed the rest of keepiq is EUPL-1.2, which .editorconfig contradicts eight lines later. - "see the WHY block further down" pointed at an unlabelled block, and the pre-existing "(see the header)" back-reference at the node-core fallbacks had become circular. Give that block a WHY THE BASE CONFIG IS INLINED heading so both references resolve. - "several of upstream's own explanatory comments" is two, measured. - REUSE.toml said both files "stay" AGPL; only .editorconfig was already so. - The copy-out warning now says why it is absolute: Conduction may relicense its own half, but nothing marks which lines are which. Comments only. The resolved webpack config is byte-identical.
…onfig chore(license): declare webpack.config.js AGPL-3.0-or-later
…ion instead of being blocked Review of #809 (🔴). The 409 for a suite that is part of an in-progress migration had no way around it for an admin. Whoever holds the session and the leaked password could start a compromise recovery, commit one record so the owner's abort is refused, and leave the migration in_progress for good. Every migration route is owner-only, so the admin's force-revoke, the containment tool, then answered 409 forever. - forceRevoke with markCompromised=true no longer checks for an open migration. After revoking the suite, it ends the migration through MigrationService::terminateInProgressForCompromise(). That sets status `terminated`, which releases the write lock, and dispatches the same aborted event the owner's abort uses, so the locked SecretRequests are released too. It then revokes the migration's other end as compromised, because either end may be the attacker's. The response names both, as `terminatedMigration` and `alsoRevokedSuite`. - Without the flag, the 409 stays, so a routine revoke still cannot strand a migration. - The Requirement now names the compromise exception and gains the adversarial scenario. - 🟢 The assertNoMigrationInProgress() docblock now says it is a check, not a lock. Mutation-checked, 5 mutations, all red: compromise still hitting the 409, the other end not revoked, the migration not terminated, finished migrations terminated too, and no aborted event. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…the source is flagged too Review of #805. - The compromise listeners sent `secretId`/`secretName`, while KeepiqNotifier reads `secret_id`/`secret_name`. The warning therefore read "Your secret "a secret" may be compromised", with no link. Both SuiteCompromiseOnRevokeListener and SuiteCompromiseListener now send snake_case params. - For a shared copy, both now point the warning (secret_id, secret_name, objectId) at the SOURCE secret, which its owner can open and has to rotate. resolveTarget() replaces resolveSourceOwner() and returns that source. - The revoke cascade now also stamps `possibly_compromised_at` on the source secret and flags it for rotation, not only the revoked user's copy. This is the half of #802 the listener reorder alone could not fix. stampAndFlag() keeps handle() under the phpmd complexity limit. - 🟢 SuiteLifecycleEventRegistrarTest now dispatches through the real Symfony EventDispatcher that Nextcloud forwards priorities to, instead of restating its ordering rule. Mutation-checked, 7 mutations, each red: camelCase params and pointing at the copy, in both listeners; the source not stamped; the source not flagged; and the registrar priority back at 0. Full unit suite passes on the host (1347). The delegation promotion on a compromise revoke, also raised in the review, is tracked separately in #817. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Resolves the conflicts on #804. They were only in the 74 l10n catalogs: both sides had appended keys to the end of each translations object. Each .json is development's version plus this branch's two emergency-carry strings, and the .js catalogs are regenerated from those with `npm run l10n:build`. After the merge: check:l10n-js and test:l10n pass (36 locales at full parity, 1173 keys). The PHPUnit unit suite passes (1382, 12 skipped), and the four affected vitest specs pass (31). Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…uite subject Part 1 of the #804 review fixes. More follows before this is pushed. - Proofs are single-use. A successful verify() consumes the nonce in the distributed cache (keepiq_proof_nonce) for the rest of its lifetime: atomic add() where the cache supports it, otherwise hasKey()+set(), as JwtAuthService does for jti. Only a fully verified proof is consumed. This closes the replay Wilco found, where a designate proof captured within its 300 s window could bring back a contact the owner had just revoked. Limitation, documented in the class docblock: without a memcache (NullCache) reuse is not detected, and with APCu only it is detected per server. - VaultKeyProofMiddleware::afterException() logs every refused proof (user, route, purpose, reason) instead of only answering 403. - New proof subject `migrationNewSuite`: the new suite of the migration named by route `id`. Wiring it into the re-envelope route and the browser is next. Tests: 3 new service tests (single use, remaining-lifetime TTL, plain cache) and 2 new middleware tests (new-suite subject, refusal logged). The unit suite passes on the host (1387). KNOWN OPEN: phpmd CouplingBetweenObjects on VaultKeyProofMiddleware (13). Still to do: move re-envelope to migrationNewSuite (controller, browser, coverage test, spec), residual-contact reasons and copy, a distinct audit event for a rotation carry, spec wording for single use, and mutation checks. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…istinct carry audit Part 2 of the #804 review fixes (part 1: 37e1571). - The re-envelope route now proves the migration's NEW key: subject `migrationNewSuite`. Every migration is a compromise recovery, and the old password may be the leaked one. With the old-key proof, someone holding the session and that password could still overwrite envelopes during the recovery (the #801 harm). The browser signs with the new AES envelope and the new password it just set. - Residual contacts now carry a reason: `unreachable`, `not_confirmed` or `break_glass_in_flight`. The form shows each group with its own copy, and only unreachable contacts get the "re-establish" prompt. An unconfirmed contact gets a plain note. An in-flight one gets a warning that this is what a contact added by someone else looks like, because that is what a planted contact is after. Two new strings, translated into the 36 required locales plus en. - A carry is audited as a new event, `emergency_access.carried` (with fromSuiteId/toSuiteId), instead of `emergency_access.granted`, so after an incident it cannot be mistaken for a fresh designation. - VaultKeyProofMiddleware suppresses CouplingBetweenObjects with a written reason: the logger took it to 13. - Specs: the vault-key-proof requirement now requires single-use proofs and names the cache limitation, plus refusal logging and a "cannot be used twice" scenario. Envelope Invalidation on Key Change now requires the new-key proof, the distinct carry audit and the no-nudge rule. Mutation-checked, 11 mutations, all red. PHP: the re-envelope back to the old key, the nonce never consumed, reuse allowed with atomic add or with a plain cache, the refusal not logged, the new-suite subject resolving the old suite, and a carry audited as a grant. JS: the proof signed with the old key, in-flight reported as not_confirmed, the form nudging every residual, and no in-flight warning. Verified: PHPUnit 1387 on the host. vitest 946 in the container; the 2 failing files fail the same way on development (shared-instance, connectionRegistry). l10n gates pass (36 locales, 1175 keys). psalm 0 errors. phpcs and phpmd clean on the changed lib/ files. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
… and really unlock SecretRequests Round 2 of the review on #809. - 🟡 Order. The compromise path used to terminate the migration and only then revoke its other end. If that second revoke failed, the migration was already `terminated`, so a retry never reached the other end, possibly the attacker's suite, and it stayed live. It now finds the open migration (findInProgressForSuite, read-only), revokes the other end, and only then terminates it (terminateForCompromise). A retry after a failure still finds the open migration. revokeSuite() refuses only `compromised`, not `revoked`, so re-revoking the first suite on a retry is fine. - 🟡 SecretRequests. The aborted event's listener called unlockAndUpdateSuite(old, old), which the service always refuses ("must differ"). The failure was only logged, so locked requests stayed locked after an owner's abort, and after a compromise termination too. This dates from 49ff8c2; this PR had only added the promise. There is now an unlock that leaves the requests on their suite (SecretRequestMapper:: unlockByEncryptionSuiteId, SecretRequestSuiteLockService::unlockInPlace), and the listener uses it. Its test now drives the REAL lock service over a mocked mapper. The old test mocked the service, which is why it stayed green. - 🟢 The Requirement's opening sentence now names the compromise exception, and a "failed containment step can be retried" scenario is added. - 🟢 New test: force-revoking the NEW end also revokes the old end. Mutation-checked, 4 mutations, all red: terminating before revoking the other end, the other end always being the new end, the listener back on unlockAndUpdateSuite(old, old), and find returning finished migrations. The unit suite passes on the host (1358), psalm 0 errors, phpcs and phpmd clean. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…annot be filled Round 3 of the review on #809 (🟡). Fixing the unlock in 1fbc3e6 meant a compromise termination put the locked SecretRequests back to `pending`, on a suite that had just been revoked as compromised, possibly one whose private key the attacker holds. The public fill page then handed out that suite's certificate, and a third party opening an old link would have encrypted their credential to it. The gap behind it is older and wider: no revoke listener touches SecretRequests, so a compromise force-revoke WITHOUT a migration already left pending requests fillable. SecretRequestPolicy::classify(), which backs both the public show() and fill(), now refuses a pending request whose suite is not `active` (revoked, compromised or gone), with the new reason `unavailable` and a 410. That covers both paths. The fill page renders it as "This request is no longer available.", translated into 36 locales plus en. The spec describes what the compromise path does with the requests and gains a scenario for a request on a revoked suite. Tests: policy (revoked, compromised and missing suites are refused; an active suite stays open), and show() and fill() through a REAL policy over mocked mappers (410, reason `unavailable`, no certificate). The fill page renders the reason. Mutation-checked: removing the suite check or ignoring the status turns them red. The unit suite passes on the host (1364), the l10n gates pass, psalm 0 errors, phpcs and phpmd clean. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
The migration compromise path warned the source owner but left the source secret unstamped and unflagged, so it never showed in the rotation and compliance views. Both compromise listeners now share stampAndFlag()/resolveTarget() through a trait. resolveTarget() treats a missing share row or source as expected and logs any other lookup failure. stampAndFlag() logs and continues, so one failing secret no longer aborts the cascade for the rest. Refs #802 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
The completion sweep stamped every residual contact 'grantor_rotation', so the Emergency Access view could not tell an unreachable grantee from one the owner left unticked or one with a break-glass in flight. The sweep now records _in_flight (requested/approved), _not_carried (grantee reachable) or the plain reason (unreachable), so the view can offer Re-establish only for the last. Refs #800 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…revoke-in-progress-migration # Conflicts: # l10n/be.js # l10n/be.json # l10n/bg.js # l10n/bg.json # l10n/bs.js # l10n/bs.json # l10n/ca.js # l10n/ca.json # l10n/cs.js # l10n/cs.json # l10n/da.js # l10n/da.json # l10n/de.js # l10n/de.json # l10n/el.js # l10n/el.json # l10n/en.js # l10n/en.json # l10n/es.js # l10n/es.json # l10n/et.js # l10n/et.json # l10n/fi.js # l10n/fi.json # l10n/fr.js # l10n/fr.json # l10n/ga.js # l10n/ga.json # l10n/hr.js # l10n/hr.json # l10n/hu.js # l10n/hu.json # l10n/is.js # l10n/is.json # l10n/it.js # l10n/it.json # l10n/lb.js # l10n/lb.json # l10n/lt.js # l10n/lt.json # l10n/lv.js # l10n/lv.json # l10n/mk.js # l10n/mk.json # l10n/mt.js # l10n/mt.json # l10n/nb.js # l10n/nb.json # l10n/nl.js # l10n/nl.json # l10n/pl.js # l10n/pl.json # l10n/pt.js # l10n/pt.json # l10n/rm.js # l10n/rm.json # l10n/ro.js # l10n/ro.json # l10n/ru.js # l10n/ru.json # l10n/sk.js # l10n/sk.json # l10n/sl.js # l10n/sl.json # l10n/sq.js # l10n/sq.json # l10n/sr.js # l10n/sr.json # l10n/sv.js # l10n/sv.json # l10n/tr.js # l10n/tr.json # l10n/uk.js # l10n/uk.json
…single-use proof docs Wilco's round 3 on #804: - The Emergency Access view offered Re-establish for every invalidated contact. The grantor's list now carries the sweep's invalidatedReason (not the grantee's incoming list), and the view offers Re-establish only for an unreachable contact, warns about one with a break-glass in flight and shows a neutral label for one the owner did not carry. - A declined contact counts as not confirmed, not as in flight. - design.md D5 and ARCHITECTURE.md now describe single-use proofs. - A no-memcache install logs, once, that proof reuse is not detected. - The new-key proof is described as binding to the party who started the migration, with the limit of the start proof. - Stale docblocks about prompting every residual contact are fixed. - A test pins that a failed proof does not consume its nonce. - The destroy and re-envelope proof attributes are single-line, so gate-16 sees their @SPEC. Refs #800 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…listener-order fix(encryption-suites): warn the owners of shared secrets on a compromise revoke
…the suite mapper Wilco's round 4 on #809: - A test runs the real SuiteMigrationAbortedListener and SecretRequestSuiteLockService::unlockInPlace() over an in-memory request, then a real SecretRequestPolicy and SecretRequestService, and asserts getByToken() and fill() refuse with 410 on a revoked or compromised old suite. - SecretRequestPolicy takes the suite mapper as a required parameter, so the suite check can no longer be skipped by leaving it out. The test sites pass an active-suite mapper. - The secret-requests spec gets the unavailable refusal as its own requirement in the admin-suite-revocation change. Refs #816 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…otation Wilco's round 4 on #804. The completion sweep classified residual contacts by reachability, the recovery form by the owner's ticks, so an unticked contact with an unreachable grantee was offered Re-establish in the Emergency Access view. The server cannot see the ticks, so it no longer guesses: a break-glass in flight is recorded as grantor_rotation_in_flight and every other residual as grantor_rotation. The view offers Re-establish for no rotation reason and still warns about an in-flight contact. The recovery form's completion screen stays the only prompt for an unreachable contact. VaultKeyProofService now requires its logger, so the missing-memcache warning cannot be dropped by a wiring change. Refs #800 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…gress-migration fix(encryption-suites): refuse to revoke a suite that is part of an in-progress migration
…oved A resumed rotation never carries emergency contacts, and its outcome had no residual list, so the completion sweep removed them without a word. resumeMigration now reads back, after completion, the contacts invalidated by the rotation away from the old suite (rotationRemovedContacts) and returns them: in flight ones as break_glass_in_flight, the rest as removed_by_rotation. Refs #800 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…ncy-and-gdpr-key-proof # Conflicts: # l10n/be.js # l10n/be.json # l10n/bg.js # l10n/bg.json # l10n/bs.js # l10n/bs.json # l10n/ca.js # l10n/ca.json # l10n/cs.js # l10n/cs.json # l10n/da.js # l10n/da.json # l10n/de.js # l10n/de.json # l10n/el.js # l10n/el.json # l10n/en.js # l10n/en.json # l10n/es.js # l10n/es.json # l10n/et.js # l10n/et.json # l10n/fi.js # l10n/fi.json # l10n/fr.js # l10n/fr.json # l10n/ga.js # l10n/ga.json # l10n/hr.js # l10n/hr.json # l10n/hu.js # l10n/hu.json # l10n/is.js # l10n/is.json # l10n/it.js # l10n/it.json # l10n/lb.js # l10n/lb.json # l10n/lt.js # l10n/lt.json # l10n/lv.js # l10n/lv.json # l10n/mk.js # l10n/mk.json # l10n/mt.js # l10n/mt.json # l10n/nb.js # l10n/nb.json # l10n/nl.js # l10n/nl.json # l10n/pl.js # l10n/pl.json # l10n/pt.js # l10n/pt.json # l10n/rm.js # l10n/rm.json # l10n/ro.js # l10n/ro.json # l10n/ru.js # l10n/ru.json # l10n/sk.js # l10n/sk.json # l10n/sl.js # l10n/sl.json # l10n/sq.js # l10n/sq.json # l10n/sr.js # l10n/sr.json # l10n/sv.js # l10n/sv.json # l10n/tr.js # l10n/tr.json # l10n/uk.js # l10n/uk.json
…ed, on every path Wilco's round 5 on #804. With the standing view's Re-establish gone, a resumed rotation removed emergency access with no word anywhere. Now: - the recovery form's completion screen names the contacts a resumed rotation removed, neutrally, with no prompt to re-establish; - the resume banner, which disappears on completion, raises a toast that stays until dismissed, with the count and a pointer to Emergency Access; - the Emergency Access view shows a text-only notice on each contact a rotation removed, which also covers a completion screen closed unread; - the grantee's incoming list labels an invalidated contact plainly, not "(re-establish)". The specs, ARCHITECTURE.md and compromise-recovery.md now describe what each path tells the owner. There are four new strings in 37 locales. Refs #800 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…eads back the removed contacts (#804 f1) resumeMigration() read back the removed contacts even when finaliseMigration() returned finalised: false. The completion sweep had not run yet, so the list was always empty. acceptMigrationLosses(), which completes that rotation afterwards, never read back at all. The owner was told nothing on either the form (Retry, then Accept) or the banner (Resume, then Open key rotation, then Accept). - resumeMigration reads back only after a completion the server accepted - acceptMigrationLosses reads back after completeMigration and returns the contacts as residualContacts - the form's handleAcceptLosses keeps the initiate path's own list when it has entries, and otherwise uses the read-back Class sweep: every caller of completeMigration (the initiate and resume paths through finaliseMigration, plus acceptMigrationLosses). Abort fires no completion listener, so it removes no contact. Thread: #804 (comment) Co-Authored-By: Claude Opus 5.5 <[email protected]>
…nreachable prompt is gone (#804 f2) handleRetry() replaced the form's result with the resume outcome. That dropped the initiate run's classified list (unreachable, not confirmed, in flight) in favour of the neutral read-back and its "your key rotation was resumed" note, although the owner never left the dialog. - handleRetry keeps the initiate list when it has entries, through the same keepResidual() rule the accept-losses path uses - spec: the recovery-form bullet also covers a completion by retrying or by accepting a loss, plus a scenario for the retry Thread: #804 (comment) Co-Authored-By: Claude Opus 5.5 <[email protected]>
… by accepting a loss (#804 f1) The spec part of the f1 fix in 0bb1981. The resumed-rotation bullet now says that a completion by accepting a loss also reads back, and a scenario pins what the completion screen shows then. Thread: #804 (comment) Co-Authored-By: Claude Opus 5.5 <[email protected]>
… runs a branch the live path never takes (#804 f6) - rotationRemovedContacts logs a failed contacts request (console.warn with the repo's no-console exception), instead of returning [] with no trace. The standing Emergency Access view still shows each removed contact. - resumeMigration reads the old suite id before completion. The real completeMigration refreshes the status to null once the migration ends, so this.migrationStatus?.oldSuiteId always fell back to oldSuite.id live, while the test (which kept the status set) only ran the first branch. - the store test's finaliseMigration mock now clears migrationStatus as the real call does, and the failed-read-back test asserts the log. Class sweep: acceptMigrationLosses already reads its old suite id before completing (f1); the initiate path does not read back. Thread: #804 (comment) Co-Authored-By: Claude Opus 5.5 <[email protected]>
…the residual" rule survive in the change (#804 f7) The rule that the sweep removes only unreachable grantees, and that the owner is prompted to re-designate every residual contact, predates keepiq#800. Now every contact the browser did not carry is removed: an unreachable grantee, one the owner did not tick, and one whose break-glass was in flight. Only an unreachable one gets the re-establish prompt. - tasks.md 3.2 and 1.6, and their copies in plan.json - tasks.md and plan.json gain 3.4 (the resume read-back and banner toast) and 3.5 (the standing-view notice). plan.json uses ids 30 and 31 so existing ids stay stable - design.md: Goals, D2 and D3 - proposal.md: the fallback-sweep bullet, the "re-designate that specific contact" bullet, and the Frontend impact line - EmergencyAccessSuiteRotationListener's comment on the sweep Class sweep: git grep -n -i "exactly the residual\|re-establish exactly\|finds only\|could not be re-enveloped\|cannot be reached" from the repo root finds only text that covers every uncarried contact. The plan.json copies, proposal.md, design.md Goals and the listener comment were not named in the finding. Thread: #804 (comment) Co-Authored-By: Claude Opus 5.5 <[email protected]>
… after Retry or Accept (#804 f1) Found by the pre-push check on the f1 and f2 commits. keepResidual() kept the initiate list as-is whenever it had entries. A contact the owner left unticked, which then got a break-glass requested while the loss acknowledgement was pending, stayed labelled "not confirmed", although the completion sweep recorded grantor_rotation_in_flight. The planted-contact warning never showed. keepResidual() still keeps the initiate classification, but takes the in-flight reason from the read-back for any contact it names. It adds no contacts the initiate list doesn't know, because those would render under the resumed-rotation copy on an initiate run. The Emergency Access view still shows them. A test pins each of these two choices. Thread: #804 (comment) Co-Authored-By: Claude Opus 5.5 <[email protected]>
fix(review): #804 review findings, round 5
…key-proof fix(emergency-access): require key proofs so a stolen session cannot reach the new private key
Release: merge development into beta
WilcoLouwerse
approved these changes
Sep 29, 2026
Contributor
Quality Report — ConductionNL/keepiq @
|
| 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 | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-29 16:03 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #395, closes #800, closes #801, closes #802, closes #803, closes #816, closes #173, closes #184.
Release:
beta(18f594c) intomain(0.3.2, released 2026-09-12). 273 commits, about 70 merged PRs.Issues this closes
The PRs below merged into
development, which is not the default branch, so theirCloseslines closed nothing. This PR carries every issue the delta actually fixes that is still open:POST /api/v1/suitesmints a second active encryption suiteEncryptionSuiteProvisioningService)createAdminHandover()has no production callerDelegationControllerandAdminHandoverPanelcall itAlready closed by hand, so not listed: #707 (#727), #793 (#807), #794 (#813), #795 (#810).
Deliberately left open:
What's in this release
Security
Platform
max-version35 (fix(apphost): Nextcloud 35 support — public-API OpenRegister autoload prelude + derived CI matrix (max-version 35) #712).doriath_tables becomekeepiq_, and 35 migrations are consolidated into one (Rename the doriath_ tables to keepiq_ and consolidate 35 migrations into one #652). Attachment blobs move to thekeepiqAppData namespace (refactor(storage): move attachment blobs to the keepiq AppData namespace #654). The machine API accepts both thekeepiqand thedoriathwire names (feat(api): accept both keepiq and doriath on the machine API wire contract #655).Features
Fixes: CLI envelope reading (#807), restoring every secret type from a backup (#812), and a count of undecryptable secrets before export or send (#813).
Housekeeping: REUSE/SPDX licensing for the whole tree (#636, #647), npm audit findings cleared (#640), dependency bumps, the parity matrix (#737 and follow-ups), OpenSpec passes (#767, #769, #781), and removal of the dead docusaurus site (#798).
Behaviour changes to flag
doriath_tokeepiq_, and attachment blobs move. Back up before upgrading.Before merging
appinfo/info.xmlon beta says0.3.4-unstable.20260912202807. Set the release version.AI assistance
Drafted with Claude Code (Claude Opus 5.5) from the merged PRs, their bodies, the issue states and a check of beta's code for #173 and #184.
🤖 Generated with Claude Code