[dj4oC]

Problem

Office documents opened in Collabora Online can be renamed but "Save As" / "Export As" silently does nothing — the copy is never written back into oCIS storage.

Root cause: Collabora implements "Save As" as a host-delegated operation. When the user triggers it, Collabora posts a UI_SaveAs message and waits for the integration to reply with the target filename via Action_SaveAs; only then does it render to the chosen format and call WOPI PutRelativeFile. The app-provider (web-app-external) never opened that postMessage channel — it never sent Host_PostmessageReady and never handled UI_SaveAs (it only listened for UI_Edit). Rename works because Collabora performs it as a native WOPI RenameFile call that needs no host involvement.

Fix

In web-app-external/src/App.vue:

• Send Host_PostmessageReady once the editor iframe has loaded — this is what makes Collabora start emitting its postMessage API (App_LoadingStatus, UI_SaveAs, …).
• Append ui_defaults=SaveAsMode=group to the editor URL so Collabora shows the grouped Save-As control that delegates the target filename to the host (the parameter is Collabora-specific and ignored by other app providers).
• Handle UI_SaveAs: prompt for the copy's name (the extension selects the export format) and reply with Action_SaveAs. Incoming messages are now origin-checked against the editor origin.

No server change is needed — the collaboration service already implements PutRelativeFile.

Testing

• Unit: web-app-external tests pass; snapshot updated for the ui_defaults URL parameter.
• check:types, ESLint clean.
• Verified live against ocis_full + Collabora Online 25.04.7.3.1:
  • PutRelativeFile writes the converted copy into the space (confirmed end-to-end: a real call created a .pdf next to the source; PutFile: success → PutRelativeFileSuggested: success, HTTP 200).
  • Sending Host_PostmessageReady on iframe load opens Collabora's full postMessage channel (App_LoadingStatus → Document_Loaded); without it Collabora stays silent. CheckFileInfo already returns UserCanNotWriteRelative:false + PostMessageOrigin.
  • The end-to-end UI_SaveAs → Action_SaveAs → PutRelativeFile chain is exercised by the office web e2e suite (webOffice), which drives Collabora's Save-As UI.

Risk

Low. Behaviour is additive: a new postMessage handshake + an extra UI param. The existing UI_Edit handling is preserved (now also origin-checked). Non-Collabora app providers ignore the ui_defaults parameter.

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[kw-security]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[DeepDiver1975]

Reviewed as maintainer. The WOPI Save-As approach is correct and well-explained: Collabora delegates "Save As" to the host via UI_SaveAs/Action_SaveAs, the Host_PostmessageReady handshake is what opens that channel, and PutRelativeFile is already implemented server-side. The refactor of the old catchClickMicrosoftEdit into a single onAppMessage switch is a clean improvement (and now correctly registers/unregisters the listener and re-evaluates openAsPreview reactively rather than once at mount). The changelog fragment is present and follows changelog/TEMPLATE. CI is green. Nice work.

I have one requested change before approving, plus a test gap.

1. (requested) Origin check fails open while appUrl is unresolved

const onAppMessage = (event: MessageEvent) => {
  if (unref(appOrigin) && event.origin \!== unref(appOrigin)) {
    return
  }
  ...

appOrigin is a computed over appUrl, which is undefined until the async loadAppUrl task resolves; on parse failure it also yields ''. Because an empty appOrigin is falsy, the && short-circuits and the origin check is skipped entirely — every origin's message is dispatched. The message listener is attached in onMounted, which fires while the WOPI open_url POST is still in flight (network latency, the 425 "still processing" path, a slow IdP). During that window any page holding a reference to this window can post {"MessageId":"UI_SaveAs"} (→ unsolicited "Save a copy" modal) or {"MessageId":"UI_Edit"} (→ forced view→write reopen of the user's own file). It also disables the check for the whole session if the backend ever returns a non-absolute app_url.

Impact is low (no cross-origin exfiltration — postToApp correctly fails closed on empty origin and never uses targetOrigin:'*'; the actions only affect the user's own file). But this is exactly the fail-open pattern the recent embed-mode CSRF fix (#13844, security-embed-mode-postmessage-origin-validation.md) set out to eliminate, so it's worth holding the same line here. The fix is to fail closed:

if (\!unref(appOrigin) || event.origin \!== unref(appOrigin)) {
  return
}

No legitimate editor message can arrive before appUrl is set (the editor iframe can't have loaded yet), so rejecting until the origin is known is strictly correct and closes both the load-race window and the bad-app_url case.

2. (test gap) No unit coverage for the new handler

The only test delta is the two updated snapshots for the ui_defaults URL param. The new behavior — origin filtering in onAppMessage, UI_SaveAs → modal → Action_SaveAs payload, and Host_PostmessageReady on iframe load — has no unit test. The PR body credits the webOffice e2e suite for the end-to-end chain, which is great, but the origin-check logic (precisely the part flagged in #1) is cheap to assert at the unit level and would lock in the fail-closed behavior so it can't regress. Please add a small test that (a) drops a message from a foreign origin, (b) accepts UI_SaveAs from the editor origin and asserts the modal/Action_SaveAs payload, and (c) asserts Host_PostmessageReady is posted on @load.

Minor (non-blocking)

• The modal accepts any filename and posts it straight into Action_SaveAs.Values.Filename. Server-side PutRelativeFile is responsible for sanitizing the name (strip path separators / leading dots), which I trust it does — but a small client-side guard against empty///\ names in onConfirm would be reasonable defense-in-depth and avoids a pointless round-trip.

Happy to approve once the origin guard fails closed (#1); the test (#2) would ideally land in the same PR. Thanks!

[dj4oC]

Thanks for the thorough review @DeepDiver1975 — all addressed in ed7188f.

1. (requested) Origin check now fails closed. onAppMessage rejects every message until the editor origin is known and accepts only that exact origin:

if (!unref(appOrigin) || event.origin !== unref(appOrigin)) {
  return
}

This closes the load-race window (the listener is attached on mount, before the open_url POST resolves) and the non-absolute app_url case. As you noted, no legitimate editor message can arrive before appUrl is set, so rejecting until the origin is known is strictly correct.

2. (test gap) Unit tests added in app.spec.ts for the new handler:

• a message from a foreign origin is dropped (no modal);
• a message arriving before the origin is known is dropped (the fail-closed path from Themability ... #1);
• UI_SaveAs from the editor origin opens the modal and, on confirm, posts Action_SaveAs with the expected payload and targetOrigin;
• invalid file names are rejected;
• Host_PostmessageReady is posted on iframe @load.

3. (minor) Client-side filename guard added. The modal now validates via onInput (live, using the repo's setError pattern) and re-checks in onConfirm: empty names and any / or \ are rejected before Action_SaveAs is posted. Server-side PutRelativeFile still sanitizes; this is defense-in-depth and avoids the pointless round-trip.

Local: check:types, lint, check:format, and test:unit for web-app-external (8 passing) are all green.

[DeepDiver1975]

Re-reviewed ed7188f9a (changed code only). Both points from my previous review are resolved — thanks @dj4oC.

1. Origin guard now fails closed. onAppMessage is now if (\!unref(appOrigin) || event.origin \!== unref(appOrigin)) return, so messages are dropped until the editor origin is resolved to an absolute URL, instead of being waved through when appOrigin is empty/unparseable. This is the fix I asked for, and the inline comment correctly captures why (listener attached on mount before the WOPI open_url POST resolves).

2. Tests added. The new app.spec.ts cases cover the security behavior directly — ignores editor messages until the editor origin is known (fail closed) exercises the unresolved-appUrl path, plus foreign-origin drop, the happy-path Action_SaveAs round-trip, the empty / path-separator filename rejection, and Host_PostmessageReady. Good coverage.

The added validateSaveAsFilename (reject empty + //\, guarded on both onInput and onConfirm, .trim() before send) is sensible defense-in-depth on top of the server-side PutRelativeFile sanitization.

No remaining concerns on the changed code. CI is green across the board. LGTM from my side.

[LukasHirt]

Ran a multi-agent review (28 verifier agents, high effort) across the two changed files. Found 4 confirmed bugs / design issues that warrant changes before merge, plus 6 plausible hazards worth noting. Grouped by severity below.

---

Confirmed (request changes)

1. withSaveAsUiDefaults silently no-ops when ui_defaults already in the URL — feature entirely broken for Collabora deployments where the server decorates the URL with its own ui_defaults (e.g. UIMode=notebookbar). searchParams.has returns true, the branch is skipped, SaveAsMode=group is never appended, Collabora never sends UI_SaveAs. Should append to the semicolon-delimited list instead of replacing.
2. onSaveAs has no isReadOnly guard — loadAppUrl already short-circuits with a visible error for read-only files, but onSaveAs dispatches the modal regardless. When the user confirms, Action_SaveAs is posted; WOPI rejects PutRelativeFile silently; the modal closes normally. User sees success, no copy is created.
3. Collabora-specific protocol applied unconditionally to all WOPI providers — withSaveAsUiDefaults and onIframeLoad → Host_PostmessageReady fire for every provider regardless of which editor is active. ui_defaults=SaveAsMode=group is appended to every app_url, and Host_PostmessageReady is posted on every iframe load. Should be gated on appName indicating Collabora.
4. validateSaveAsFilename duplicates shared validator logic — already diverges from getNameErrorMsg in useFileActionsCreateNewFile/useFileActionsRename (those also reject ., .., and reserved names). Adding a third implementation means they drift further over time.

Confirmed (test gap)

5. No test that UI_Edit is a no-op when openAsPreview is false — the old static guard prevented this case entirely; now it's a conditional inside the switch. If the guard is accidentally inverted, any authenticated Collabora instance can force a write-mode reload of any file. No existing test would catch the regression.

Plausible hazards (worth addressing)

6. Relative app_url silently breaks UI_Edit — appOrigin computes '' for relative URLs; every postMessage (including UI_Edit) is then dropped, so the Edit button stops working for any deployment where the WOPI server co-hosts on the same origin as the web app.
7. Silent postToApp null + modal dismissed as success — if loadAppUrl restarts between UI_SaveAs receipt and the user pressing Save, appIframe can be transiently null; postToApp no-ops silently but ModalWrapper still closes the dialog, giving false positive feedback.
8. onConfirm closure over live reactive refs — in a SPA where the component is reused across resources without remounting, a resource-prop change while the modal is open rewrites appUrl/appIframe; the user's Save press dispatches Action_SaveAs to the new document.
9. UI_SaveAs can double-fire — no re-entrancy guard; rapid double-click of Collabora's export button opens two concurrent Save As dialogs and sends two Action_SaveAs messages, causing two PutRelativeFile calls.
10. flushAppUrl hardcoded to 5 ticks instead of flushPromises — if loadAppUrl gains another async step the count silently falls short, making tests intermittently unreliable.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[LukasHirt]

Re-reviewed after fabe69253 and b48d42e03. All confirmed issues from my previous review are addressed, plus the plausible hazards I flagged:

Confirmed (were blocking):

1. ui_defaults append bug — fixed, now appends to the existing semicolon-delimited list instead of skipping.
2. Missing isReadOnly guard on onSaveAs — fixed.
3. Collabora-specific behavior applied to all providers — fixed, gated behind an isCollabora computed.
4. validateSaveAsFilename duplication — not a literal getNameErrorMsg reuse (neither implementation is a clean fit; see inline reply), but the character-level gap is closed by adding ./.. rejection.
5. Test gap for UI_Edit no-op — fixed, test added.

Plausible hazards (also addressed):
6. Relative app_url permanently breaking the origin check — fixed via window.location.origin base.
7. No re-entrancy guard on UI_SaveAs — fixed.
8. Silent no-op / stale-closure risk on postToApp/onConfirm — fixed, postToApp now returns success and the target/origin are captured at dispatch time.
9. Hardcoded 5-tick flushAppUrl — replaced with flushPromises().

Local check:types, lint, check:format, and test:unit for web-app-external are all green (13 passing in app.spec.ts).