Addresses multi-agent review findings on the uploaded-plugin nodeExecution
consent gate:
- id validation: use an allowlist (/^[A-Za-z0-9][A-Za-z0-9._-]*$/) instead of a
Unicode denylist, closing bidi/zero-width/homoglyph dialog-anchor spoofing the
range list missed (U+061C, U+2060, U+3164, fullwidth chars); strip all Unicode
control+format chars from the self-declared display name/version.
- never upgrade trust: describeVerifiedBuiltInDialog returns null on any imperfect
on-disk verification (id mismatch, missing permission, unreadable manifest) and
the grant handler falls back to the unverified dialog, so a colliding uploaded id
can never borrow a built-in's verified dialog.
- reserve the gitea/linear/trello/azure-devops issue-provider bundled ids (they had
drifted out of BUNDLED_PLUGIN_IDS, leaving an impersonation gap) and guard the
BUNDLED_PLUGIN_PATHS subset-of BUNDLED_PLUGIN_IDS invariant with a node test.
- key the revoke and exec IPC handlers through the same assertSafePluginId as the
grant handler so the "revoke by id on teardown/re-upload" guarantee can't drift.
- clear the session nodeExecution denial when a plugin is uninstalled, so a fresh
re-upload of the same id is prompted again rather than silently failing closed.
- de-duplicate the PluginNodeExecutionElectronApi interface into a single
electron/shared-with-frontend model (was copied byte-identically in two files).
Multi-agent review of the Phase 1 gate found the uploaded plugin id —
used as a path component in the bundled-manifest existsSync probe — was
not rejecting path separators or dot-segments. Impact was bounded (the
verified-builtin branch re-validates with the strict kebab regex before
any read, so no code exec / file read / dialog spoof), but `..` / `/`
left a filesystem-existence oracle and rendered misleadingly as the
dialog "Plugin ID". assertSafePluginId now rejects `/`, `\`, `.`, `..`.
Also from review: factor the shared Allow/Deny dialog shell, and correct
the denied-cache comment (the existing token short-circuit handles the
multi-call-site case; the cache only makes a denial sticky across a later
non-interactive grant re-entry). Documents the uploaded-id constraints.
Refs #8512
Re-open the nodeExecution permission for uploaded/community plugins
(previously built-in only, #8205) behind the existing main-process
consent dialog. Phase 1 of #8512; unblocks the Super Productivity MCP
plugin (discussion #8385).
- Main process sanitizes the attacker-controlled plugin id and the
self-declared name/version before they reach the consent dialog or
the grant map (control/bidi/whitespace rejected, length-capped).
- Bundled vs uploaded is decided by the on-disk manifest, never a
renderer-supplied flag, so uploaded code can't borrow a built-in
plugin's verified name.
- Uploaded-plugin dialog anchors on the validated id, flags the plugin
as unverified third-party with full machine access / no sandbox, and
defaults to Deny.
- Revoke is main-authoritative by (pluginId, webContents) so a re-upload
reusing an id can't inherit a live session grant.
- Consent stays session-scoped: an in-memory, never-synced denied set
prevents re-prompt storms; deny keeps the plugin enabled but fails
node calls closed until re-enable or restart.
Refs #8512#8385
* fix(op-log): lock snapshot save to prevent lost-update window
saveCurrentStateAsSnapshot() read NgRx state then lastSeq without
holding OPERATION_LOG lock. An op appended between the two reads would
get seq <= lastAppliedOpSeq but its effect would be absent from the
snapshot. On next hydration the tail replay would start after that seq,
silently skipping the op forever.
Fix: wrap in lockService.request(LOCK_NAMES.OPERATION_LOG, ...) and
read lastSeq BEFORE state snapshot so the worst interleaving degrades
to harmless re-replay (idempotent) rather than a missed op.
Fixes#8308
* fix(op-log): address review feedback on snapshot lock PR
- Amend JSDoc idempotency claim: syncTimeSpent is additive on re-replay
- Add inline note about compaction's opposite read order and worse failure mode
- Add lock regression tests (#8308): lock acquired, read order, error handling
Co-Authored-By: Claude <noreply@anthropic.com>
* refactor(sync): remove unused error classes, dialog, and constructor-time logging
Phase 1 of #8325: clean up orphaned sync error types and their UI.
Removed error classes that are no longer thrown anywhere:
- NoEtagAPIError, FileExistsAPIError (unused API errors)
- RevMismatchForModelError, SyncInvalidTimeValuesError (superseded by file-based flow)
- RevMapModelMismatchErrorOnDownload/Upload, NoRemoteModelFile, NoRemoteMetaFile
- LockPresentError, LockFromLocalClientPresentError, MetaNotReadyError, InvalidRevMapError
Removed DialogSyncErrorComponent and all references in SyncWrapperService
(_forceDownload, _handleIncoherentTimestampsDialog, _handleIncompleteSyncDialog,
_openSyncErrorDialog, _extractModelIdFromError).
Removed constructor-time logging from JsonParseError, ModelValidationError,
DataValidationFailedError — these errors are logged at the catch site; redundant
construction-time logs risk leaking user data.
Cleaned up dead translation keys (D_INCOMPLETE_SYNC block, DIALOG_RESULT_ERROR,
ERROR_DATA_IS_CURRENTLY_WRITTEN) from en.json and t.const.ts.
Updated file-based-sync-flowchart.md to reflect the removed error types.
---------
Co-authored-by: Claude <noreply@anthropic.com>
Linux StatusNotifierItem hosts redraw the whole tray item on every
Tray.setImage(), so cycling the 16 progress-animation frames made the
icon flicker between a full and a partial ring. Show a single static
running icon on Linux; Windows/macOS keep the animation.
Also re-render the static stopped/running icons (light+dark, @2x) from
vector sources at 24/48px so GNOME no longer upscales a 16px source.
#7097 force-disabled the custom title bar on all GNOME and hid the
toggle, even though only GNOME+Wayland actually fails to render the
Window-Controls-Overlay. This stranded GNOME-X11 users (where the
feature worked) with no way to re-enable it.
- Narrow the kill-switch from IS_GNOME_DESKTOP to a new IS_GNOME_WAYLAND
(XDG_SESSION_TYPE/WAYLAND_DISPLAY, mirroring idle-time-handler). GNOME
on X11 keeps the feature and the settings toggle.
- Unify GNOME detection: preload imports it from common.const instead of
re-implementing, so main and renderer can't drift.
- Align global-theme.service with main-window: force native decorations
on GNOME+Wayland in both, so a persisted/synced `true` no longer lays
the custom header over native controls (doubled header).
- Add unit tests for the detection matrix.
A Windows tray-icon GUID binds to the executable's full path unless the
binary is code-signed with an organization in the signature subject. The
Store (MSIX) and portable/scoop builds run from versioned directories
whose path changes on every update, so the GUID goes stale and
Shell_NotifyIcon(NIM_ADD) fails silently (Electron raises no JS error).
new Tray(image, guid) still returns a live Tray, so the try/catch and
ensureIndicator() truthiness guards never detected the failure — the
window hid to an invisible, unreachable tray.
Use the path-bound GUID only for the stable-path NSIS build; create the
tray without a GUID on Store and portable, so Windows identifies the icon
by window handle and it stays visible across updates. The NSIS path is
untouched, preserving its taskbar-pin persistence.
Refs: https://www.electronjs.org/docs/latest/api/tray (guid)
* fix(keyboard): resolve macOS global shortcut layout mismatch (#8378)
* fix(keyboard-layout): log layout-detection failure and resolve layoutReady with map copy
* fix(keyboard-shortcut): remove debug console logs and add macOS scope comment
* fix(keyboard-shortcut): preserve modifier separator when mapping plus key shortcut
* refactor(keyboard-shortcut): export mapping helpers and avoid as any cast in configuration mapping
* test(keyboard-shortcut): add unit tests for layout shortcut translation logic
* test(keyboard-shortcut): use correct KeyboardConfig type instead of as any in test fixture
* docs(keyboard-shortcut): hoist macOS physical shortcut layout comments to helper JSDoc
* refactor(config): extract global shortcut keys and mapping helpers
* test: add test helper capability for IS_ELECTRON and IS_MAC
* test: add comprehensive integration unit tests for global shortcut effects
* feat(electron): eagerly trigger layout detection on Electron startup for macOS timing fix
* refactor(config): InjectionToken migration, layout detection hardening, and startup optimization
* refactor: address non-blocking suggestions for layout detection and DI tokens
* build(electron): move keyboard-config.model to shared-with-frontend to fix ASAR require
* fix(client-id): increase entropy to 6 chars and fix related test regressions
The GHSA-hr87 scheme allowlist (#8210, v18.10.0) permitted only
http/https/mailto/file, which silently broke custom app deep-links such
as obsidian:// in task-attachment links and markdown notes.
Add low-risk app-launch + standard schemes (obsidian, vscode,
vscode-insiders, zotero, logseq, tel, sms) to the shared allowlist. This
restores the links across all sinks (OPEN_EXTERNAL, OPEN_PATH, the
navigation fallback, and the markdown link renderer) while still blocking
the OS-level handlers (ms-msdt:, search-ms:) the allowlist exists to stop.
If a folder pick (setSyncFolderPath) completes while the very first
getSyncFolderPath disk read is still in flight, the load's resolution
callback would overwrite the freshly-set cache with the now-stale disk
value (null), leaving sync wrongly not-ready until restart. Only seed
the cache from disk when it's still unset; the freshly-picked value wins.
Resolve conflict in electron/local-file-sync.ts by keeping the PR side:
master's assertPathOutside guard was added inside the TO_FILE_URL handler,
which this PR deletes entirely (deletion supersedes the guard). Also drop
master's two new TO_FILE_URL tests (the handler no longer exists).
Pre-merge review of #8228 flagged four real items in the otherwise
solid security boundary. All non-blocking; rolled into one pass:
1) IMAGE_PICK_AND_IMPORT was the only renderer-callable handler that
threw raw Errors back across IPC. The FS handlers all funnel
through createSafeIpcError, which strips e.stack so main-bundle
paths can't leak via the renderer's error-handling. Switch
IMAGE_PICK_AND_IMPORT to the same shape: returns Error on
validation failure, null on user cancel, value on success.
Renderer adapter uses `instanceof Error` (parity with fileSyncLoad
et al). New test asserts the returned error has no path content
and no stack.
2) The image picker button was not disabled while the IPC was in
flight. A second click would queue a second IPC, opening a second
dialog after the first resolves; because `replacesId` is computed
from the form value at click time, the first import's id would
never be GC'd. Add an `isPickerBusy` signal and bind the button's
`disabled` to it. New spec asserts the second concurrent click is a
no-op and the busy flag clears in the `finally`.
3) Stale comment in `local-file.model.ts` pointed at
`electron/sync-folder-store.ts`, which doesn't exist (the storage
was inlined into local-file-sync.ts in Phase 2.5). Update to
describe the actual current shape and clarify that the field is
read once on upgrade for the migration breadcrumb and never written
from new code paths.
4) Image input placeholder advertised `file://` URLs as a valid
input. Phase 4 removed the IPC that resolved them, so the
placeholder was a footgun. Drop the `file://` half.
68/68 electron security tests; 8/8 formly-image-input karma; lint +
tsc clean.
End-to-end review of the #8228 sweep flagged one real residual risk
and several smaller items. This commit addresses them in one pass.
The IMAGE_CACHE_IMPORT IPC accepted a renderer-supplied absolute path
with no link to a recent SHOW_OPEN_DIALOG signal. A compromised
renderer could call importImage() with any image-extension path under
5 MB outside userData — same information-disclosure shape the legacy
READ_LOCAL_IMAGE_AS_DATA_URL had, just one-shot instead of repeatable.
Fix: merge the dialog and the import into a single atomic IPC,
IMAGE_PICK_AND_IMPORT. Main opens the picker inline and only calls
importImage() with the file the user actually clicked. The renderer
no longer holds a path at any point in the flow; an attacker who
controls the renderer cannot trigger an image read without a real
user dialog interaction.
Shape change:
- Return null on user-cancel; throw on validation failure. This
preserves the existing UX where cancel is silent and an unreadable
image shows a snack. Renderer-side openFileExplorer is a try/catch.
- Optional { replacesId } arg garbage-collects the previously
cached image when the user picks a new one. Closes the disk-fills
follow-up the review raised (#3) without an extra IPC round-trip.
Smaller items from the review folded in:
- Extract _resolveRelative helper in local-file-sync.ts; the five
FS handlers had the same three-line incantation copy-pasted. The
Phase 2.5 simplification over-corrected by inlining everything.
- Drop the dead __resetSyncFolderCacheForTests export; no caller, and
the tests reset via require.cache surgery anyway.
- image-cache.ts getExt now uses path.extname so Windows backslash
paths are handled correctly (the old slash-only split worked by
accident).
- resolveBgImageToDataUrl: short-circuit on empty id, and log a
console.warn when a legacy file:// is detected (no resolvable IPC
remains; user must re-pick). i18n snack is a documented follow-up.
- Package LocalFileSyncElectron.isReady now logs a critical line via
the injected sync logger when isReady=false but the renderer's
privateCfg still has the legacy syncFolderPath — dev console
surfaces the "needs re-pick" state without dragging i18n into the
security PR.
- Stale comment in electron-file-adapter.ts referenced a deleted file
(sync-folder-store.ts is inlined into local-file-sync.ts now).
Tests:
- electron/local-file-sync.test.cjs gains four IPC-level cases:
legacy IMAGE_CACHE_IMPORT not registered, pick+import round-trip,
cancel returns null, validation failure throws, replacesId GCs the
prior cached file.
- formly-image-input.component.spec rewritten for the new flow:
asserts imagePickAndImport is called with replacesId when one is in
state, snack on rejection but not on null, no setValue on either.
68/68 electron security tests pass; 7/7 formly-image-input karma;
5/5 resolve-bg-image karma; 17/17 sync-providers package
local-file specs. Lint + tsc clean.
What this commit does NOT do (called out so it isn't forgotten):
- CLIPBOARD_COPY_IMAGE_FILE has the same renderer-supplied-path shape
for clipboard images. Parallel issue; out of scope for #8228.
- PICK_DIRECTORY has no defaultPath — could nudge users toward a
dedicated sync subfolder vs picking ~/Documents. Cosmetic.
- Symlink TOCTOU between resolveSyncPath and writeFileSync stands;
O_NOFOLLOW-on-open is the proper fix and needs cross-platform care.
- Migration UX is console-only; a user-visible snack needs i18n
infrastructure not appropriate for the security commit.
With Phase 3's image cache in place, the picker no longer hands the
renderer an absolute path or produces file:// URLs. The two legacy
IPCs that still accepted renderer-supplied absolute paths are now
dead surface area:
- READ_LOCAL_IMAGE_AS_DATA_URL — read any image-extension file
outside userData and inline it as base64. The renderer could
swap the stored value for any matching path between renders.
- TO_FILE_URL — convert any path outside userData into a file://
URL the renderer could embed. Was the precondition for the
above.
Both are removed entirely (handlers, preload bindings, type
signatures, IPC enum entries). The remaining image flow is:
showOpenDialog (proven user intent)
→ imageCacheImport(path) — main copies into private cache
→ renderer stores `image:<id>`
→ imageCacheGetDataUrl(id) per render
Migration: existing configs that hold `file://` values from older
builds render no background until the user re-picks. The resolver
util explicitly detects the legacy shape and returns null with a
comment pointing to this commit. Same migration model we used for
the sync folder (Phase 2): a one-time re-pick is the cost of
closing the security gap, and the picker UI is already in place.
Also drops the now-stale `IS_ELECTRON` early-return in the resolver
util — `window.ea?.imageCacheGetDataUrl` is undefined on web,
short-circuits naturally, and the implicit check made the spec
unable to exercise the Electron branch.
Tests updated: assertion that the legacy handlers are no longer
registered (a compromised renderer that still tries the IPC must
see no handler), and a positive `image:<id>` round-trip in the
karma util spec. 65 electron security tests + 5 util spec all
green; lint + tsc clean.
The background-image picker previously gave the renderer an absolute
path to an arbitrary image-extension file and stored it as a file://
URL in user config. The render path then asked main to inline that
file:// on every theme apply. Main only refused paths inside userData,
so a compromised renderer could swap the stored value to any png/jpg/
gif/webp/svg path outside userData and re-read it for as long as it
stayed in config.
Switch the picker to copy-to-cache, serve by opaque id:
- electron/image-cache.ts (new): main-owned directory at
`userData/bg-images/<id>.<ext>`. importImage(absolutePath) validates
the source (outside userData, allowed extension, 5 MB cap, non-empty,
is-file), copies it in, and returns a `randomBytes(16)` hex id. The
id is unguessable so a renderer cannot iterate the cache directory
for files it didn't import. getImageDataUrl(id) reads the cached
file and returns a `data:<mime>;base64,…` URL, or null.
- electron/local-file-sync.ts: two new IPCs, IMAGE_CACHE_IMPORT and
IMAGE_CACHE_GET_DATA_URL, registered next to the existing image
handlers.
- electronAPI.d.ts + preload.ts: imageCacheImport / imageCacheGetDataUrl
bindings.
- formly-image-input.component: after showOpenDialog returns a path,
call imageCacheImport(path) and store `image:<id>` in the form
control. Surface the existing "couldn't read image" snack on
rejection. Removes the toFileUrl round-trip and removes the
absolute-path leak into user config.
- resolve-bg-image-to-data-url.util: handle the `image:<id>` prefix by
calling imageCacheGetDataUrl. Legacy `file://` values are still
accepted for back-compat (the existing userData-guarded
READ_LOCAL_IMAGE_AS_DATA_URL handles them); since the picker no
longer produces new file:// values, the long-tail drains as users
re-pick. A future commit can remove the legacy code path.
Tests:
- electron/image-cache.test.cjs: 12 cases — happy path, in-userData
rejection, bad extension, too-large / empty / missing source,
non-string input, unknown / malformed id, cross-restart persistence,
removeCachedImage.
- electron/local-file-sync.test.cjs: 3 new IPC-level cases for
IMAGE_CACHE_IMPORT (round-trip, userData refusal) and
IMAGE_CACHE_GET_DATA_URL (unknown id).
- formly-image-input.component.spec: rewritten to assert
imageCacheImport is called and `image:<id>` is what lands in the
form, plus a failure-path snack assertion.
67 of 67 electron security tests pass; 6 of 6 formly-image-input
karma tests pass; lint and tsc clean.
Post-review of Phase 2 (#8228). The reviewer flagged one real bug and
several pieces of premature scaffolding.
Real bug — finding #1: the sync form was still writing the sync folder
path back into the renderer credential store. Phase 2 made the IPCs
take a relative path and main own the canonical root, but the Formly
button at sync-form.const.ts:200 had `key: 'syncFolderPath'`, which
caused FormlyBtn.onClick to setValue(picker-return-string) on the
form model. That flowed into privateCfg.syncFolderPath on save. The
"renderer no longer holds the authoritative copy" claim was true in
the IPC contract but untrue at runtime. A compromised renderer could
keep mutating its own privateCfg.syncFolderPath; nothing on master
reads it after Phase 2, but any future code path that does would
re-open the hole. Fix:
- drop `syncFolderPath` from PROVIDER_FIELD_DEFAULTS[LocalFile] so it
is not in the LocalFile credential-store defaults at all
- drop `key: 'syncFolderPath'` from the LocalFile picker button so the
picker's return value is not bound to the form model
KISS simplifications (a/b/d from review):
- delete electron/sync-folder-store.ts and its test; inline the
load/cache/persist helpers at the top of local-file-sync.ts as
~25 LOC. The dedicated module was carrying an init/get/set ceremony
and a Promise singleton for a single string that is read at most a
few times per second in the worst case. simple-store is the durable
layer either way.
- drop the `_resolveOrThrow` typeof precheck; resolveSyncPath already
type-checks `relativePath` and throws the same opaque error
- inline `resolveSyncPath(...).absolutePath` into each handler; the
helper was the third layer doing the same thing
Other findings addressed:
- finding #2 (PICK_DIRECTORY silently undefined on persist failure):
PICK_DIRECTORY now returns a safe Error via createSafeIpcError when
realpath/persist fails, distinct from `undefined` for user-cancel.
New tests cover both branches plus the "cache must not be poisoned
with a non-existent path" invariant.
- finding #4 (set-without-canonicalize): the inlined
setSyncFolderPath now `realpathSync.native`s the picked path before
persisting, so a relative or symlinked picker result is rejected
early and the on-disk form matches what resolveSyncPath compares
against on read.
- finding #5 (fire-and-forget init that was also being awaited per
handler): dropped the fire-and-forget; handlers just `await
getSyncFolderPath()`, which lazy-loads from disk on first call and
caches.
All 52 electron security tests still pass; lint clean; tsc clean.
Phase 2 of #8228. Closes the renderer-supplied-absolute-path hole: the
file-sync IPCs now take a relative path; main resolves it against a
sync folder it owns and never trusts the renderer's copy of.
Main side
- sync-folder-store: persists the path in simple-store (still inside
userData, still renderer-unreachable via assertPathOutside) with an
in-memory cache so per-IPC lookup doesn't stat the file every call.
initSyncFolderStore() is fire-and-forget at adapter init; each handler
awaits it defensively because it's idempotent via _loadOnce.
- FILE_SYNC_SAVE/LOAD/REMOVE: accept { relativePath, ... }, pass through
resolveSyncPath, write/read/delete the vetted absolute path.
- CHECK_DIR_EXISTS and FILE_SYNC_LIST_FILES: accept an optional
relativePath; default to the sync root itself so the existing
"is the sync folder reachable?" check still works.
- PICK_DIRECTORY: now persists the chosen folder via setSyncFolderPath
before returning the display string to the renderer. The renderer
receiving the string is no longer load-bearing — it's display only.
- New GET_SYNC_FOLDER_PATH IPC returns the current value for display
(settings form, "sync folder: …" labels).
- TO_FILE_URL: pulls in the assertPathOutside(userData) backstop that
was missing on master, so a userData path cannot be laundered into a
file:// URL that later flows through READ_LOCAL_IMAGE_AS_DATA_URL or
the navigation guard.
Package (@sp/sync-providers/local-file)
- LocalFileSyncElectron.isReady now asks main (getMainSyncFolderPath
dep) instead of reading the renderer's privateCfg. This drives the
migration UX: an existing install with a legacy privateCfg value but
no main-side path lands on "not configured" and gets re-prompted by
the picker. A one-cycle "please pick again" is the migration cost; we
decided against silently promoting the renderer's persisted value
because it could be tampered with.
- getFilePath returns the *relative* path (no absolute folder prefix).
The leading-slash strip is preserved so existing call sites that
prepend '/' still work.
- pickDirectory no longer writes to renderer privateCfg — main owns
persistence. The dep contract still returns the display string.
- The privateCfg.syncFolderPath field is marked @deprecated; the
field is kept for migration of older configs and may be removed in a
later pass once the migration has rolled out.
Renderer
- ElectronFileAdapter forwards the relative path verbatim to the IPC.
- The renderer's LocalFileSyncElectron deps wire pickDirectory and the
new getMainSyncFolderPath to the bridge.
Tests
- electron/local-file-sync.test.cjs rewritten for the new signatures:
happy path, traversal escapes, absolute-relativePath rejection,
sync-folder-equals-userData rejection (grant-file forgery defense),
PICK persists main-side, GET returns the configured path, TO_FILE_URL
refuses userData.
- electron/sync-folder-store.test.cjs covers persistence,
init-before-get throw, null/empty handling, idempotent set.
- The package's local-file-sync-electron.spec.ts is updated for the new
deps shape and asserts the package no longer writes to the renderer
credential store.
All 56 electron-side tests pass; 374 sync-providers tests pass.
* fix(electron): lock in-window navigation to the app's loaded origin
The previous will-navigate handler accepted any URL whose hostname was
'localhost' or '127.0.0.1', which left the privileged main window
reachable by any local web server (e.g. http://127.0.0.1:1337/) — the
preload bridge (window.ea) would have been exposed to whatever page that
server returned. In production the legitimate origin is file:// only, so
there is no excuse for ever navigating to http://localhost in-window.
Compare against the URL the app actually loaded (captured at loadURL
time) instead of a hostname allowlist. http/https requires exact
host+port match; file:// requires the same html pathname; data:/blob:/
javascript: are rejected outright. The same guard is also applied to
will-redirect so a same-origin start cannot be redirected onto an
attacker page mid-navigation, and a defensive did-create-window handler
destroys any unexpected child window (the deny-all setWindowOpenHandler
should make that path unreachable).
Also tightens TO_FILE_URL with the same userData deny used by the other
file-sync IPCs, so a plugin/XSS cannot launder a userData path into a
file:// URL that later flows through READ_LOCAL_IMAGE_AS_DATA_URL.
* fix(electron): reject UNC/remote-host file:// in navigation guard
`new URL('file://192.168.1.100/Applications/SP.app/Contents/Resources/index.html').pathname`
equals the local app's pathname, so the previous pathname-only check
considered an attacker-controlled UNC host same-origin and let it load
in the privileged main window. The app's loaded file:// URL is always
local (empty host), so require `target.host === ''` explicitly.
* test(electron): add userinfo-@-trick navigation guard regression
`http://localhost:4200@evil.com/` parses with host=evil.com and
username=localhost — a naive substring or startsWith check would be
fooled. The existing host-equality check already rejects it; lock that
in with a test so a future refactor cannot regress.
Phase 1 of the fix for #8228, simplified after KISS review.
The earlier draft (commit 5cb83b41a, since reverted) introduced a
multi-grant capability store with opaque grantIds and a feature
discriminator. Review showed the abstraction was overhead: this app
has exactly one user-pickable filesystem location (the sync folder).
Backgrounds will go through copy-to-cache, not folder grants;
plugins do not get folder grants; backups use a main-derived path.
And the opaque grantId added zero marginal security over a featureless
{relativePath} API since an XSS in the renderer would just replay the
id from memory. The security boundary is "main owns the canonical root
and the resolver logic", not the id.
So: drop the store, keep the resolver. resolveSyncPath takes
(syncFolderPath, relativePath, userDataDir) and returns the vetted
absolute path. Layered defenses:
- syncFolderPath canonicalized every call (folder may have moved
since startup); missing/inaccessible root denies
- canonical root must not be userData or live inside it (a user who
picks userData as their sync folder would otherwise hand the
renderer authority over settings/grants/db)
- relativePath may be '' or '.' for LIST/CHECK ops on the root;
anything else must not be absolute and must not escape
- leaf symlinks refused (v1 policy; O_NOFOLLOW alternative is a
follow-up that needs cross-platform care)
- canonical resolved path (or deepest existing ancestor when the
leaf doesn't exist yet) must still live inside the canonical
root — catches symlinks/junctions in intermediate directories
and case-fold / 8.3-shortname aliases
- ancestor walk fails CLOSED on EACCES; only ENOENT lets us walk
up. A permission-restricted ancestor must not silently allow.
Errors are opaque (PathNotAllowedError, no path in message) and the
stack is stripped — IPC handlers further sanitize before returning to
the renderer.
Phase 2 will wire this into FILE_SYNC_SAVE/LOAD/REMOVE/LIST_FILES and
CHECK_DIR_EXISTS, move syncFolderPath ownership from the renderer's
privateCfg to a main-side simple-store key, and add a re-confirm
migration on first launch.
16 unit tests cover the rejection cases from #8228's test plan plus
the EACCES-fail-closed branches that came out of the KISS review.
Lays the groundwork for issue #8228: today the FILE_SYNC_* IPCs accept
any renderer-supplied absolute path outside userData, which means any
plugin or XSS in the renderer can read/write arbitrary user files
(~/.ssh, /etc/passwd, etc.). The goal is to replace string-path
authorization with capability-based grants: the renderer holds only an
opaque grantId, the main process holds the canonical root, and FS ops
take (grantId, relativePath).
This commit adds the main-side primitives but does not wire them into
any IPC yet:
- grant-store: persisted, main-only, feature-scoped grants backed by
simple-store under a new 'fileGrants' key. Opaque hex ids from
crypto.randomBytes. Roots are canonicalized at create time, and
revalidated at startup so a folder that was moved, deleted, or
swapped for a symlink-elsewhere invalidates the grant cleanly.
- grant-resolver: resolveGrantPath(grantId, relativePath, feature)
returns the absolute path the IPC handler may touch. Enforces the
feature scope match, rejects absolute paths and '..' traversal, and
refuses symlinks. For not-yet-existing leaves it canonicalizes the
deepest existing ancestor so a directory symlink mid-path cannot
sneak a write outside the root. Errors are opaque
(PathNotAllowedError) and never embed the offending path.
Notes on tests: ts-node's transpile-only loader does not downlevel
'for...of map' iteration, so the store uses Array.from(map.entries())
where it iterates the cache. The existing electron *.test.cjs use the
same loader and the same workaround was already implicit in their
patterns.
Phase 2 (switching FILE_SYNC_SAVE/LOAD/REMOVE/LIST_FILES + CHECK_DIR_EXISTS
to grant-based, with re-confirm migration of the existing syncFolderPath)
and Phase 3 (image picker copy-to-cache) follow in separate commits.
will-navigate gated on url.includes('localhost'), so hosts like https://localhost.evil.com (or a 'localhost' substring in the query) were treated as the app's own origin and navigated the main window directly, bypassing the openUrlInBrowser scheme guard. Parse the URL and match the hostname exactly (localhost/127.0.0.1).
No regression for prod: it serves from file:// and routes via hash (withHashLocation), so will-navigate never fires for the app's own origin; only real external links reach this handler.
A sweep for auto-loading subresource sinks fed attacker-controllable
(synced) content found two more no-click NTLM vectors beyond the
markdown/attachment images already gated:
- READ_LOCAL_IMAGE_AS_DATA_URL (electron/local-file-sync.ts): a synced
theme backgroundImage 'file://host/share/x.png' reaches this main-process
handler, where fileURLToPath -> fs.stat/readFile on the resulting UNC path
opens an SMB connection and leaks the NTLM hash — no click, just the theme
being active. This is the most reliable variant (main-process fs, not a
Chromium subresource). Guard with isPathSafeToOpen before any fs access.
- note.imgUrl (note.component): a synced note image URL is bound directly
to <img [src]>/[enlargeImg] and auto-loads on render. Gate at the setter
so only a safe URL reaches the bindings.
Both reuse the existing isPathSafeToOpen helper; local file://, http(s),
data: and blob: are unaffected. New electron test proves stat/readFile are
never reached for UNC / remote file:// input.
(cherry picked from commit 80a3c20b3f5a3f535f5d04d7fd215ba569493ef8)
* fix(plugins): harden node execution grants
* fix(plugins): harden iframe bridge boundaries (#8208)
* fix(plugins): harden iframe bridge boundaries
* fix(plugins): tighten iframe bridge follow-up
* test(plugins): make node-executor electron test hermetic
The new plugin-node-executor test read the real built-in plugin manifest
(src/assets/bundled-plugins/sync-md/manifest.json), which is a build
artifact absent when 'npm run test:electron' runs in CI (before the
frontend/plugin build). Stub the manifest read, scoped to the executor
module via Module._load, so the grant/token/webContents assertions no
longer depend on built plugin assets.
* fix(plugins): allow iframe formatDate & getCurrentLanguage i18n methods
The iframe API allow-list gate added in #8208 only listed a subset of
the i18n methods that master's #8146 exposes to iframe plugins. Without
this, plugin calls to formatDate/getCurrentLanguage (and translate) are
rejected with 'Unknown API method'. Add all three so the merged gate
matches the methods createBoundMethods/createPluginApiScript expose.
* docs(plugins): document node-exec handoff bootstrap-ordering invariant
The one-shot consumePluginNodeExecutionApi() handoff is defended by
construction ordering, not structural isolation: PluginBridgeService must
consume it before any plugin 'new Function' code runs (both share window.ea
in one renderer realm). Document the invariant at the consumption site so a
future lazy-service/early-plugin-load refactor can't silently regress it.
Surfaced by the post-merge security review (latent finding; not currently
exploitable).
* fix(plugins): use static iframe sandbox attribute to avoid NG0910
Binding a security-sensitive iframe attribute (sandbox) via [attr.sandbox]
makes Angular throw RuntimeError NG0910 and tear down the iframe, crashing
the plugin view to the global error screen. This broke the plugin-iframe,
plugin-loading and plugin-lifecycle e2e tests.
Restore the static sandbox attribute (still without allow-same-origin, so
the opaque-origin isolation from #8208 is preserved) and drop the now-unused
iframeSandbox binding/import.
* fix(security): canonicalize file-path-guard and restrict FILE_SYNC_* to outside userData
file-path-guard now canonicalizes via fs.realpathSync.native (deepest existing
ancestor) before the path.relative check, so symlinks and case-insensitive / 8.3
filesystem aliases can't bypass containment — this also hardens the existing
backup-dir (GHSA-x937) check, which was purely lexical and bypassable on macOS.
Adds assertPathOutside and applies it to every FILE_SYNC_* handler and
READ_LOCAL_IMAGE_AS_DATA_URL in local-file-sync.ts, so the renderer (untrusted
plugin/XSS) can no longer read/write/delete/list/inline files inside the app's
private dir (userData: settings/grants/db) while user-chosen sync folders still
work.
* fix(security): validate clipboard image basePath, fileName and copy source
ipc-handler-wrapper now uses isPathInsideDir containment (not a bare startsWith,
which accepted a sibling like <userData>-evil) and rejects any fileName that is
not a plain image basename or imageId containing separators/.. — closing the
clipboard grant-file forgery (fileName='simpleSettings'). CLIPBOARD_COPY_IMAGE_FILE
now requires an image-extension source. The supported-image-extension allowlist
is consolidated into a single SUPPORTED_IMAGE_EXTENSIONS in mime-type-mapping.
* fix(electron): validate URL scheme before shell.openExternal (GHSA-hr87-735w-hfq3)
Task-note Markdown links had their href passed verbatim to shell.openExternal
on click, letting note content (sync, imported backups, issue-provider data)
silently invoke any OS-registered protocol handler — ms-msdt: (Follina),
search-ms:, \\host\share (NTLM hash capture), etc. — with one click and no
prompt. Angular's sanitizer does not help: its SAFE_URL_PATTERN blocks only
javascript:, so these schemes pass.
Add a shared scheme allowlist (http/https/mailto/file) enforced at both layers:
- marked renderer (marked-options-factory): blocked-scheme links render as
inert <span> text instead of an anchor (covers all note render paths + web).
- Electron main process, both shell.openExternal sinks: the OPEN_EXTERNAL IPC
handler (system.ts) and openUrlInBrowser (main-window.ts, the
will-navigate/window-open path used by the fullscreen/archived dialogs).
The allowlist lives in electron/shared-with-frontend so renderer and main agree
on one policy. Blocked links are styled muted/non-interactive with an
explanatory tooltip.
* fix(electron): harden openExternal/openPath against SMB & attribute-injection (GHSA-hr87-735w-hfq3)
Follow-up hardening from multi-agent review of the initial fix:
- file:// SMB bypass: `file:` was allow-listed unconditionally, so
`file://host/share` / `file:////host` reached shell.openExternal — the same
NTLM-leak vector the raw `\\host\share` check blocked. Restrict `file:` to the
canonical local form `file:///<path>`. Gated on the raw string because
Chromium (renderer) and Node (main) parse file: hosts/paths differently.
- IPC.OPEN_PATH was unguarded: a synced FILE-attachment path of `\\host\share`
leaked NTLM via shell.openPath. Reject UNC paths (new isUncPath helper).
- Attribute-injection XSS: the marked link renderer interpolated href/title
raw; marked's angle-bracket link form could inject an event handler that runs
under disableSanitizer. HTML-escape href/title.
- Remove the now-dead/misleading explicit backslash check and redundant
toLowerCase (new URL already throws on raw UNC; protocol is spec-lowercase).
Adds cross-engine-verified tests for file:// SMB forms, isUncPath, and the
href quote-injection case.
* fix(electron): block remote file:// in OPEN_PATH too (GHSA-hr87-735w-hfq3)
Re-review found the OPEN_PATH guard was incomplete: isUncPath only rejects raw
slash-prefixed paths, but shell.openPath('file://host/share') resolves to
\\host\share on Windows — the same NTLM leak. A synced FILE-attachment path is
attacker-controllable.
Centralize the path policy in isPathSafeToOpen() (rejects UNC paths and remote
file:// URLs, allows local paths and file:///) so the two sinks can't drift.
* fix(electron): gate image src against remote file:// / UNC (GHSA-hr87-735w-hfq3)
Markdown image src auto-loads on render (no click), so a remote
file://host/share or UNC src silently triggers an SMB connection and
leaks the user's NTLM hash just by viewing a synced/imported note — the
same vector the link/openExternal gate already blocks, but with no user
interaction. CSP (img-src ... file:) does not block it.
Gate image href through isPathSafeToOpen: remote file:// and UNC srcs
render as inert (HTML-escaped) alt text instead of an <img>; local
file://, http(s), data: and blob: images are unaffected.
Also drop a dead eslint-disable directive on ALLOWED_EXTERNAL_URL_SCHEMES
(the name does not trip naming-convention).
* fix(tasks): gate IMG attachment src against remote file:// / UNC (GHSA-hr87-735w-hfq3)
An IMG-type task attachment renders <img [src]="resolvedOriginalPath">,
which auto-loads on render with no click. A synced/imported remote
file://host or UNC path there silently triggers an SMB fetch and leaks
the user's NTLM hash just by opening the task — the same no-click vector
the markdown-image gate already blocks, but via Angular template binding
(Angular's URL sanitizer does not help; SAFE_URL_PATTERN allows file:).
Drop unsafe srcs via isPathSafeToOpen so they never reach the binding;
local file://, http(s), data: and blob: images are unaffected.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* feat(electron): add global shortcut for task widget toggle
Adds a configurable global shortcut (`globalToggleTaskWidget`) that
shows/hides the task widget without changing the persisted
enabled/disabled preference. The shortcut is a no-op while the task
widget feature is disabled.
When the user reveals the widget via the shortcut while the main window
is visible, a sticky "user-forced visible" flag keeps it up (like
always-show) instead of letting the next focus/show event immediately
hide it. The flag clears when the user toggles the widget off, opens the
app from the widget, or the feature is disabled.
Rebased onto master to drop the now-superseded tray-indicator refactor;
the only main-window.ts change is the additive user-forced-visible gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(electron): handle task widget shortcut races
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(e2e): retry supersync time-estimate dialog until input binds
The fill('10m') could fire its `input` event before the duration input's
value-accessor (an `input` HostListener) was wired, so the typed value
never reached the model and submit() saved an empty time — the panel
stayed "-/-" and toContainText('10m') timed out (scheduled run
27197862391, SuperSync 1/6). Retry the whole open→fill→submit cycle so a
fill that didn't stick is re-applied once the control binds.
* fix(electron): restrict loadBackupData IPC to the backup directory
The BACKUP_LOAD_DATA handler passed the renderer-supplied path straight
to readFileSync with no validation. Because plugin background scripts run
via `new Function` in the renderer, any installed plugin (or an XSS
payload in task content) could read arbitrary files through
window.ea.loadBackupData('/etc/shadow').
Constrain the path to BACKUP_DIR / BACKUP_DIR_WINSTORE via a new
isPathInsideDir guard. It uses path.relative (not a startsWith string
compare) so `..` traversal is collapsed and a name-prefixed sibling
directory (backups vs backups-evil) is not mistaken for a child. The only
legitimate caller already passes paths built from BACKUP_DIR, and
getBackupPath() is display-only, so backup restore is unaffected.
Adds electron/file-path-guard.test.cjs (node --test) covering traversal
escape, prefix-sibling, absolute-outside, and normalize-stays-inside.
Refs GHSA-x937-wf3j-88q3
* docs(electron): note lexical-only containment in file-path-guard
Clarify that isPathInsideDir does string-based containment (no fs.realpath),
so a symlink planted inside the dir is out of scope — it requires local
filesystem write access, outside this guard's renderer-input threat model.
Surfaced during multi-agent review of the GHSA-x937-wf3j-88q3 fix.
* test(electron): load guard via computed path for asar require check
The electron-smoke job runs tools/verify-electron-requires.js over the
packaged app.asar, which flags literal relative require() calls that can't
resolve in the package. The new guard test had require('./file-path-guard.ts'),
but .ts source is excluded from app.asar, so the check failed. Load the module
via a computed path (require(path.resolve(__dirname, ...))) — the same pattern
the other electron *.test.cjs files use; the scanner skips computed requires.
Also reworded the comment, which had reintroduced the literal pattern (the
scanner matches raw text, comments included).
* fix(ui): prevent stored XSS in enlarge-img directive
The enlarged-image element was built by interpolating the image URL into an innerHTML string, so a crafted synced/imported note.imgUrl could break out of the src attribute and inject an event handler (stored DOM-XSS). Build the <img> with createElement and property assignment instead, so the URL is never parsed as HTML. Adds a regression spec.
Refs: GHSA-78rv-m663-4fph
* fix(plugins): authorize nodeExecution from main-process state
The Electron main process authorized Node execution from the manifest the renderer passes on each IPC call, and window.ea.pluginExecNodeScript is callable by any renderer code — so injected renderer JS could forge {permissions:['nodeExecution']} and run arbitrary Node (CWE-501).
The main process now keeps its own grantedPlugins set, populated via a dedicated PLUGIN_SET_NODE_CONSENT channel. The renderer registers a grant in _fireOnReady (before the first node call, covering every load path including zip upload) and revokes it on teardown. The executor requires set membership.
Defense in depth, not a hard boundary: the registration channel is itself renderer-reachable, so a fully-compromised renderer could register a grant itself. It blocks the disclosed PoC (forged manifest for a never-loaded plugin) and removes per-call manifest trust. A hard boundary needs a main-process consent dialog.
Refs: GHSA-78rv-m663-4fph
* fix(security): add object-src 'none' and document CSP constraints
Adds object-src 'none' (the app embeds no <object>/<embed>) and replaces the stale TODO with an accurate note on why 'unsafe-eval'/'unsafe-inline' cannot be dropped yet: the plugin runtime and the inline bootstrap script use new Function, and the packaged app loads the renderer over file:// (opaque origin), so tightening script-src to 'self' is unreliable.
Refs: GHSA-78rv-m663-4fph
* fix(macOS): gracefully quit app
When macOS App is closed via menu item "Quit", make sure that IPC flow completes first.
- close main window instead of directly invoking `quitApp()`
* fix(macOS): use quit-intent flag
- do not touch "minimizeToTray" setting
- instead set "quitRequested" flag to indicate quit intent
- set timeout to cancel quit request, if app fails to respond within 5 seconds
* refactor(macOS): split quit-intent state and add timeout rationale
- Separate the dual-purpose timer handle into an explicit boolean +
named timer to make the semantics obvious.
- Extract the 5s window as a named constant with a comment on why this
particular trade-off (long enough for sync/finish-day, short enough
to not block a subsequent tray-hide click after cancel).
- Fix the misleading "set flag" comment in the no-window branch
(no flag is set there; none is needed).
- Note that the `closed` reset clears the pending timer so it doesn't
keep the event loop alive.
Pure cleanup, no behavior change.
---------
Co-authored-by: Johannes Millan <johannes.millan@gmail.com>
* feat(sync): integrate onedrive with pkce and operation log sync
* fix(sync): add oauth state validation and token refresh concurrency control
* fix(sync): refactor onedrive oauth cleanup and reduce download API calls
- Replace setInterval-based OAuth state pruning with on-demand
_pruneExpiredOAuthStates() to avoid background timer overhead
- Optimize downloadFile to read ETag from content response headers
first, falling back to a metadata request only when needed (reduces
API calls from 2 to 1 in the common case)
- Narrow OneDrive class interface from SyncProviderServiceInterface to
FileSyncProvider for stronger type guarantees
- Accept optional devPath constructor parameter for non-production
folder namespacing
- Extract isOneDriveClientIdRequired to a helper function in
sync-form.const.ts for readability
- Change default OneDrive sync folder name to 'Super Productivity'
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 review feedback for OneDrive sync
Critical fixes:
- Detect invalid_grant in token refresh responses and clear credentials
- Reset _tokenRefreshInFlightPromise in clearAuthCredentials to prevent
stale refresh results from overwriting cleared state
- Re-read config before persisting refreshed tokens to avoid race with
concurrent clearAuthCredentials calls
Important fixes:
- Extract OAuth state management to shared oauth-state.util.ts to fix
circular dependency between OneDrive provider and callback handler
- Add 401 retry pattern (_is401Retry) matching Dropbox's approach:
on 401, proactively refresh token and retry, only clear credentials
if retry fails
- Remove unused auth status translation keys (AUTH_SUCCESS,
BTN_AUTHENTICATE, BTN_REAUTHENTICATE, REAUTH_SUCCESS,
STATUS_CONFIGURED, STATUS_NOT_CONFIGURED) and form props
- Remove unused requireAuth parameter from _cfgOrError
Minor fixes:
- Replace path-exposing log messages with structured logging
- Remove unused _formatHttpErrorDetails helper
- Return empty ETag fallback in getFileRev instead of throwing
- Add folder cache invalidation comment for path changes
- Add afterEach to restore global fetch in tests
- Add PKCE defence-in-depth comment in auth code dialog
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): use eslint-disable-next-line for @microsoft.graph.conflictBehavior
Replace spread+computed-property workaround with a plain property plus
an explicit eslint-disable-next-line comment. Clearer intent: the
naming violation is a Graph API requirement, not accidental.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): pre-save OneDrive form config in reauth() to prevent MissingCredentialsSPError
Re-auth was failing silently on Linux because getAuthHelper() reads
clientId from IndexedDB (privateCfg), but the form values were never
written before calling configuredAuthForSyncProviderIfNecessary().
- Extract shared BTN_REAUTHENTICATE and REAUTH_SUCCESS translation keys
- Add OneDrive to OAUTH_SYNC_PROVIDERS
- Pre-save form config in reauth() matching the existing save() flow
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): adapt TooManyRequestsAPIError to new constructor signature after rebase
* refactor(sync): migrate OneDrive provider to packages/sync-providers
Move OneDrive core logic into the shared sync-providers package,
matching the pattern used by Dropbox, WebDAV, and other providers.
The app-side code is now a thin adapter with a createOneDriveProvider()
factory function.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): re-throw MissingRefreshTokenAPIError from _requestOAuthToken catch block
The catch block in _requestOAuthToken was swallowing the
MissingRefreshTokenAPIError thrown after clearing credentials on
invalid_grant, causing it to fall through to a generic HttpNotOkAPIError
instead. Now re-throws MissingRefreshTokenAPIError explicitly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 second review critical and important items
- Replace _is401Retry instance field with per-call parameter to prevent
concurrent 401 races (Critical #1)
- Fix _requestOAuthToken catch block to re-throw MissingRefreshTokenAPIError
instead of swallowing it (Critical #2)
- Fix _parseOAuthCallback defaulting unknown provider to 'unknown' instead
of 'dropbox' to prevent misrouting (Critical #3 + Important #6)
- Change conflictBehavior from 'replace' to 'fail' to prevent data loss
if a file exists with the same name as the folder (Critical #4)
- Await in-flight token refresh promise in clearAuthCredentials before
clearing, to close the refresh-during-clear race (Important #5)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 review important and minor items
- Extract duplicate OneDrive pre-auth save block into
_persistOneDriveFormCfgBeforeAuth() method (Important #7)
- Add GET probe in _ensureSyncFolderExists to skip per-segment
creation when folder already exists (Important #9)
- Redact sensitive params (code, code_verifier, refresh_token,
access_token) from token endpoint and Graph error bodies
(Important #11)
- Remove targetPath from _mapAndThrow error messages to prevent
logging user content (Important #12)
- Add OAuth state validation in manual paste flow for OneDrive
(Important #13)
- Fix unknown provider routing in _parseOAuthCallback (Important #14)
- Guard Electron IPC listener against post-destroy emissions
(Important #15)
- Remove dead authStatus Formly field from sync-form.const.ts (Minor)
- Simplify Headers builder in _ensureSyncFolderExists (Minor)
- Fix spec afterEach to restore original fetch instead of undefined
(Minor)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): fix clearAuthCredentials race and add unit tests for OneDrive
Fix a race condition in clearAuthCredentials where _tokenRefreshInFlightPromise
was nulled AFTER awaiting, causing the invalid_grant handler's clearAuthCredentials
to wait on itself. Now captures the promise and nulls the field before awaiting.
Add 5 new unit tests covering critical code paths:
- 401 token refresh and retry
- 401 retry failure with credential clearing
- 400 invalid_grant credential clearing
- 412 precondition failed mapping
- Concurrent token refresh deduplication
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 third review items
- Simplify clearAuthCredentials: null _tokenRefreshInFlightPromise without
awaiting to avoid deadlock when called from inside the refresh IIFE
- Add superproductivity:// scheme validation in Electron OAuth listener
- Fix invalid_grant test to assert MissingRefreshTokenAPIError
- Fix folder cache test to match GET-probe optimization
- Set setComplete default in beforeEach
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test(sync): fix OAuthCallbackHandler spec to expect 'unknown' provider
The _parseOAuthCallback now returns 'unknown' instead of 'dropbox' for
URLs without an explicit provider path segment.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 fourth review items
- Handle generic 401 from token endpoint (clear creds + MissingRefreshTokenAPIError)
- Remove user-supplied paths from error constructors (rule 9 compliance)
- Soften clearAuthCredentials comment to reflect actual TOCTOU guarantee
- Tighten Electron scheme gate to match native path prefix
- Add scheme info to Electron rejection log
- Fix folder-cache test to assert GET probe count
- Remove redundant per-test setComplete stub
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 fifth review items
- Scope invalid_grant/401 credential clearing to refresh_token grant only
(bad auth code exchange no longer wipes existing credentials)
- Redact OAuth code/state from Electron protocol handler logs
- Add @sp/sync-providers/onedrive TS path alias to tsconfig files
- Default undefined sync config fields instead of overwriting with undefined
- Handle OneDrive in provider-switch branch (preserve encryptKey)
- Update sync-config test expectations to match default values
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 sixth review items
- Use If-None-Match: * for null-rev uploads to prevent concurrent overwrite
- Guard in-flight refresh against stale credentials (match refreshToken/clientId/tenantId)
- Gate OneDrive provider option behind Electron/Native (web CORS unsupported)
- Don't clear credentials on transient token endpoint errors (429/5xx)
- Preserve null syncProvider instead of defaulting to WebDAV
- Hydrate full OneDrive privateCfg on provider switch
- Remove duplicate syncInterval/isManualSyncOnly Formly controls
- Revert non-English locale changes (zh.json, zh-tw.json)
- Redact URL fragments (#) in Electron protocol handler logs
- listFiles rethrows non-404 errors instead of swallowing all
- Update 401 test expectations for new rethrow behavior
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): guard credential clearing against stale concurrent refresh
Prevent a stale in-flight refresh from wiping newer credentials after
a concurrent re-auth. The refresh IIFE now throws a plain Error (not
MissingRefreshTokenAPIError) when it detects config drift, and all
clearAuthCredentials() call sites now verify the stored config still
matches before clearing. Also fix listFiles() to handle 404 from
_requestJson (which throws HttpNotOkAPIError, not RemoteFileNotFoundAPIError).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 eighth review items
- Replace If-None-Match:* with @microsoft.graph.conflictBehavior=fail query param for upload create-only semantics
- Add identity change detection in _persistOneDriveFormCfgBeforeAuth to clear tokens when useCustomApp/clientId/tenantId change
- Extract _clearIfConfigMatches helper to guard credential clearing against stale concurrent refresh
- Restrict folder probe fallthrough to 404 only, propagate other errors
- Restore locale files from upstream master
- Centralize IS_ONEDRIVE_SUPPORTED in onedrive-auth-mode.const.ts, use in factory and form config
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): address PR #7523 ninth review items
- Paginate listFiles via @odata.nextLink to avoid silent data loss
- Fix logger misuse (log -> normal with proper string message)
- Replace as any with OneDrivePrivateCfg in dialog-sync-cfg
- Throw NoRevAPIError when uploadFile response lacks eTag
- Omit undefined optional booleans from global config to prevent overwrite
- Move OneDrive dynamic import inside IS_ONEDRIVE_SUPPORTED gate
- Require state param for OneDrive full-URL paste (CSRF)
- Add id_token to sensitive keys for body redaction
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(sync): add OneDrive type alias to electron tsconfig paths
The @sp/sync-providers/onedrive path alias was missing from
electron/tsconfig.electron.json, causing the Electron build to fail
with "Cannot find module '@sp/sync-providers/onedrive'". The alias was
already present in tsconfig.base.json but electron uses its own paths.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* feat(layout): experimental vertical action strip on right edge
Teleports main-header's action-nav-right to document.body on desktop so
it escapes any ancestor containing-block, then pins it as a vertical strip
at the viewport's right edge via fixed positioning. Reserves the column
with padding-right on .app-container so the right-panel ends to its left.
* feat(layout): right-panel spans full viewport height
Restructure app shell so right-panel is a direct flex child of
.app-container (wrapping main-content via ng-content), letting its
.side extend over the full height of the viewport — including the
header area — rather than starting below the header.
* style(layout): give vertical action strip an elevated surface bg
* style(layout): match vertical action strip styling to left side nav
* fix(layout): stack play/focus buttons vertically in action strip
* style(layout): use solid surface bg for vertical action strip
* fix(layout): keep current-task title visible beside the action strip
In the teleported vertical action strip the current-task title pill was
positioned right:100% of the play button and clipped by the strip's
overflow, and the 48px mini-fab overflowed the 48px rail and got
side-clipped.
Render the title as a position:fixed flyout to the left of the strip
(the strip has no transform/filter so it is not a containing block for
fixed descendants), aligned to the play button across the web /
mac-titlebar / obsidian-header / RTL variants, and drop the play button
wrapper's horizontal margin so it fits the rail.
* fix(layout): stop tooltip overlay from blocking action strip clicks
A tooltip shown 'below' a button in the vertical action strip lands
directly over the next button. The tooltip's cdk-overlay-pane wrapper
(unlike the inner mat-tooltip-component) had no pointer-events:none, so
it intercepted clicks for real users and for Playwright actionability —
breaking ~20 focus-mode/break e2e tests.
* test(focus-mode): use teleport-robust focus-button locator
The action nav is teleported out of <main-header> into a body-level
strip, so 'main-header focus-button button' no longer matches. Drop the
main-header ancestor; 'focus-button button' is unique either way.
* fix(layout): polish vertical action strip (flyout, spacing, bg)
- Current-task title is now a hover-reveal flyout: hidden until the play
button (or the flyout itself) is hovered. Mobile (< 1080px) unchanged
(component still display:none's it there).
- Drop the position:fixed + magic top calc; keep the component's own
absolute + translateY(-50%) so the flyout is pixel-perfectly centred
on the play button regardless of theme/Material density.
- Strip now overflow: visible so the flyout can extend past the 48px
rail (overflow-x can't be visible while overflow-y is auto).
- Re-declare --header-nav-button-gap on the strip: it is scoped to
<main-header>'s :host but the strip is teleported to <body>, so the
var was undefined => every strip button had gap: 0 (cramped).
- Extra margin between the play button and the focus button.
- Strip background now matches the left sidenav (--sidenav-bg).
* fix(layout): uniform vertical spacing between strip buttons
Drop the extra play-button and counters-group bottom margins; one
--header-nav-button-gap (var(--s)) now drives the spacing between every
button in the strip. Measured: all consecutive gaps = 8px.
* feat(layout): make vertical action bar a configurable opt-in
Adds misc.isVerticalActionBar (default off). The right-edge vertical
action strip was previously an unconditional experiment; it is now an
opt-in toggle in Settings > Misc that switches the layout live without
reload:
- app.component: @if branches the DOM between classic (horizontal
header) and vertical (right-panel hoisted to full viewport height);
.app-container gets .has-vertical-action-bar to gate CSS.
- main-header: one-shot ngAfterViewInit teleport replaced with an
effect() that teleports/restores the action nav reactively to the
config flag and the desktop/mobile breakpoint.
- app/right-panel SCSS: strip padding and full-height right-panel are
now scoped to .has-vertical-action-bar; classic layout restored as
the default.
* fix(layout): even vertical-strip spacing for panel-button wrappers
plugin-header-btns, plugin-side-panel-btns, desktop-panel-buttons and
user-profile-button are zero-height wrapper custom-elements. As direct
flex children of the vertical action strip the column gap landed on the
collapsed wrapper instead of the button(s) inside, so their icons were
unevenly spaced next to real buttons like the add button. Make each
wrapper a centered column flex item that stacks its button(s) with the
same gap, and hide truly-empty wrappers so they don't reserve a phantom
gap slot. Verified: all 7 visible strip buttons now 8px apart.
* fix(right-panel): clip transient content overflow during slide animation
.side animates width 0<->* while .side-inner keeps its min-width, so
content briefly spills past the (intentionally overflow:visible) .side
during open/close. Add an isPanelAnimating host class driven by the
@slideRightPanel start/done callbacks and clip :host for that window
only (overflow: clip — no scroll container). Mirrors the existing
resizing/windowResizing transient-state pattern. Verified: class +
clip present only during the ~200ms animation, visible when idle.
* refactor(layout): drop dual DOM for vertical action bar
Single classic layout for both modes; strip is teleported and offset by
--bar-height so the horizontal header keeps owning the title-bar zone
(native drag region + WCO + Mac traffic lights). Drops the 40px
title-bar-collision padding and the right-panel full-height override.
* feat(layout): right-panel side spans full viewport height
Move the header into right-panel's projected .content slot and make
right-panel host full-height. The panel column now starts at viewport
top with the header band only spanning the .content width, similar to
Obsidian / Linear / VS Code.
* fix(layout): only offset vertical strip below WCO band on Win/Linux
Default top inset is now 0 (web, Mac hiddenInset, native-frame Electron
all let the strip start at viewport top — none of them have a window-
control overlay clashing with the right edge). Push the strip down by
--bar-height only on Win/Linux Electron with custom title bar, where
the WCO buttons would otherwise sit on top of the strip's first row.
* style(electron): use compact 32px WCO band on Win/Linux
Shrinks the Windows Controls Overlay height from 44px to 32px so the
native min/max/close buttons sit in a slimmer band — matches VS Code /
Edge slim title bar conventions and frees more of the header zone for
app content. Width stays OS-controlled (~138px); only height is
configurable.
* style(layout): tighten WCO band to 24px and pull strip up to match
Drops WCO_HEIGHT to 24px on Win/Linux and introduces a --wco-height CSS
var mirroring it. The vertical action strip now clears the WCO band by
exactly --wco-height instead of --bar-height (48px), reclaiming the
newly-freed top zone on the right edge.
* style(layout): add breathing room above strip below WCO band
Strip top inset moves from --wco-height to --wco-height + --s so the
first action button isn't flush against the bottom of the window
controls overlay on Win/Linux.
* fix(styles): scope vertical-action-bar tooltip rule to the experiment
The pane-level `pointer-events: none` was needed only because tooltips
in the teleported vertical strip can land over the next button. Apply
it via a new `body.isVerticalActionBar` body class (toggled from
`misc.isVerticalActionBar` in GlobalThemeService, alongside the existing
`isObsidianStyleHeader` effect) so unrelated app tooltips keep their
default pane behaviour.
* refactor(styles): extract vertical-action-bar CSS to its own partial
Move the 130-line experimental strip block out of `src/styles.scss` and
into `src/styles/components/_vertical-action-bar.scss`. Wired via
`_components.scss`. No CSS rules change — pure code organisation.
* merge(master): resolve _components.scss conflict in right action bar branch
Agent-Logs-Url: https://github.com/super-productivity/super-productivity/sessions/1941dd45-72e1-4a3e-92da-912fcfcb9bed
Co-authored-by: johannesjo <1456265+johannesjo@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: johannesjo <1456265+johannesjo@users.noreply.github.com>
The previous 200 KB limit silently rejected legitimately-sized
wallpapers, leaving users with a blank background and no UI feedback
(only a terminal error). Raise the limits per ingestion path:
- Electron file:// picker: 5 MB. Only the path is persisted; image
bytes are read locally on demand and never sync.
- Browser/Android file input: 256 KB. Bytes embed as a data URL and
sync via op-log, so the cap stays tight to protect SuperSync
snapshot size against per-entity bloat.
Closes#7716
* fix(maxOS): quit app on shutdown
- closing all windows alone will not quit the app on macOS
- add call to quitApp() for macOS
- this will fix the issue with the app interrupting macOS shutdown
- works now, too: Dock > Super Productivity > Quit
* fix(macOS): unset "minimize to tray" to actually close window on quit
- if "minimize to tray" is on, main window is always hidden instead of closed
- this prevents app from being quit
* Update electron/start-app.ts
---------
Co-authored-by: Johannes Millan <johannesjo@users.noreply.github.com>
Append a short channel marker to the (display-only) version string so bug
reports and the config footer reveal which build a user runs, e.g.:
win nsis W portable P MS Store MS
mac dmg D App Store MAS
linux AppImage AI snap SN flatpak FP deb/rpm L
android Play A F-Droid AF ios I web WB
Desktop channels are detected in the Electron process (process.platform,
process.mas/windowsStore, APPIMAGE/SNAP/FLATPAK_ID) via a shared helper
reused by the tray-GUID logic and exposed sync over the preload bridge
(no IPC, getAppVersionStr stays synchronous). Mobile/web are detected in
the frontend. deb vs rpm is not distinguishable at runtime, so both
report L. No caller semver-compares this string.
The macOS tray icon was loaded as a static black or white PNG chosen
once at startup from `nativeTheme.shouldUseDarkColors`, so it stayed
black on a dark menu bar (or white on a light one) after the appearance
changed.
Use the `-l` (black-on-transparent) icons on macOS and mark them as
template images. macOS then auto-inverts them for the current menu bar
appearance, including the highlight state when the menu is open and
accessibility inverted-colors. Linux and Windows behavior is unchanged;
`--force-dark-tray` is now a no-op on macOS because template images make
manual forcing unnecessary.
* feat(plugins): add onReady() API with IPC ping + fix consent write delay #7326
- Remove setTimeout(5000) from _getNodeExecutionConsent; write consent immediately
- Add plugin.onReady(fn) to PluginAPI — fires after plugin.js evaluation and IPC bridge confirmation
- Add _pingNodeBridge() in plugin.service.ts with 3-attempt retry (1s, 2s delays)
- Add triggerReady() and pingNodeBridge() to PluginRunner
- Show snack + set error state if IPC bridge unavailable after retries
- Add NODE_EXECUTION_BRIDGE_UNAVAILABLE translation key
- Add focused tests for onReady, triggerReady, pingNodeBridge, consent persistence
- Update plugin-development.md with onReady usage and nodeExecution guidance
* test(plugins): fix unused variable lint errors in spec files
* fix(plugins): guard triggerReady on instance.loaded; fix doc numbering
* fix(plugins): remove _triggerReady from public API, route ping via bridge, add retry tests
* fix(electron): add paths for @sp/sync-providers subpath exports (node moduleResolution compat)
* fix(plugins): centralize onReady, tear down runtime on activation error, add iframe onReady
Address review on #7578:
- All plugin load paths (startup, upload, reload, lazy) now go through _fireOnReady,
ensuring the IPC ping + onReady fire on every successful load — not just lazy.
- activatePlugin error path now unloads the plugin runtime (hooks, buttons, side
effects) before setting status='error', preventing partially-running plugins.
- Iframe PluginAPI now exposes onReady (fires on next microtask after plugin.js
evaluates), matching the host-side contract for typed iframe plugins.
* fix(plugins): clean up half-loaded plugins on onReady error, test real retry util
Self-review followups:
- _fireOnReadyWithCleanup wraps the 3 non-activatePlugin load paths and tears
down the plugin (unloadPlugin + remove from list + status='error' + snack)
if the IPC ping or onReady callback throws. Previously, those paths only
logged and rethrew, leaving partially-running plugins.
- Extracted retry loop into pure pingWithRetry utility; spec now exercises
production code instead of an inline-replicated stub. Removed the old
plugin-ping-node-bridge.spec.ts which was just testing its own copy of
the logic.
- Documented iframe onReady semantic (fires on microtask, no ping) in both
the source comment and docs/plugin-development.md, since cold-boot is not
a concern for iframe plugins (rendered on demand).
* ci(plugins): use npm i for root install to tolerate override drift
The root lockfile pins app-builder-lib's transitive minimatch via the
`overrides` field. npm 10.9.7 (bundled with Node 22 in setup-node@v6)
flags this as drift and fails `npm ci`, while npm 11 accepts it.
ci.yml's main test job uses `npm i`, which tolerates the drift without
mutating the lockfile on disk.
Plugin-Tests has been red on every PR since 2026-05-08 for this reason.
The inner `npm ci` for plugin-specific deps stays strict.
* fix(plugins): make onReady optional, assert callback isolation in spec
- packages/plugin-api/src/types.ts: mark onReady? optional on the public
PluginAPI interface so existing plugin TypeScript typings (and any
third-party PluginAPI implementations) remain assignable after upgrade.
The host runtime already treats onReady as optional (no-op if no
registration callback is provided), so this aligns the type with the
actual contract.
- src/app/plugins/plugin-runner.spec.ts: the previous isolation test only
asserted that triggerReady() resolved for both plugins; it would still
pass if triggerReady fired every registered callback. The updated test
wires per-plugin Jasmine spies through globalThis (the same context the
plugin code's `new Function` runs in) and asserts call counts before
and after each triggerReady, actually proving isolation.
* refactor(plugins): test real consent logic; scope startup snacks; tighten ping timeout
Address review feedback on PR #7578:
- Extract consent decision into pure `decideNodeExecutionConsent` util so the
spec exercises real code instead of a reimplemented stub. Delete the
stub-based plugin-consent.spec.ts and plugin-fire-on-ready.spec.ts (the
latter was orchestration glue already covered by plugin-runner.spec.ts and
ping-with-retry.util.spec.ts).
- Reduce per-ping timeout 5000ms -> 1500ms. Worst-case cold-boot bridge-down
detection drops from ~17s to ~7.5s; in-process vm script returning true
doesn't need 5s.
- Add PLUGIN_LOAD_FAILED translation wrapping plugin name + error. Strip the
now-redundant pluginName from NODE_EXECUTION_BRIDGE_UNAVAILABLE.
- Scope activation-failure snack to manual activations only — startup
auto-activation failures stay silent (plugin tile shows error state).
_handleReadyFailure still snacks unconditionally since onReady failure
leaves a partially-loaded runtime that the user needs to see.
---------
Co-authored-by: Benjamin <1159333+benjaminburzan@users.noreply.github.com>
Co-authored-by: Johannes Millan <johannes.millan@gmail.com>
* refactor(sync): tighten extracted package surfaces
Combined polish from the post-extraction review:
- sync-core: strip NgRx-shaped types from EntityConfig/EntityRegistry;
expose host extensions via generic param. Move StateSelector,
PropsStateSelector, SelectByIdFactory, SelectById, EntityUpdateLike,
EntityAdapterLike to a new app-side entity-registry-host.types.ts.
- sync-core: mark OpType.SyncImport/BackupImport/Repair as @deprecated;
hosts should use createFullStateOpTypeHelpers().
- sync-providers: resolve provider.types.ts vs provider-types.ts
duplication; inline implementation into the dashed canonical name.
- sync-providers: drop unused root barrel and "." export; consumers
already use focused subpath barrels (/dropbox, /webdav, etc.).
- sync-providers: replace wildcard "@sp/sync-providers/*" tsconfig path
alias with 11 explicit subpath entries matching package.json exports;
deep-internal imports now fail at typecheck.
- sync-providers: move @sp/sync-core from dependencies to
peerDependencies (kept in devDependencies for tests).
- both packages: add composite: true to enable project references;
introduce tsconfig.build.json overlay so tsup DTS bundler still works.
- gitignore: ignore **/*.tsbuildinfo composite outputs.
* refactor(sync-core): prune 47 unused barrel exports
Removes exports with zero consumers outside the package. Source files
are unchanged; only the public barrel is trimmed. Covers compression
helper classes, sync-file-prefix error/config types, replay coordinator
internals, remote-apply result types, upload/download planning option
and plan types, ports misc, conflict-resolution helper types, and
sync-import-filter decision types.
* refactor(sync-core): drop unused encryption migration path
decryptWithMigration and DecryptResult had no host consumer; they
exposed a structural-migration entry point ("here is your ciphertext
re-encrypted under Argon2id") that nothing in the codebase reads. The
side-channel setLegacyKdfWarningHandler — which IS used — stays.
encryptWithDerivedKey/decryptWithDerivedKey lose their export keyword
and remain as module-internal helpers; encrypt/decrypt/encryptBatch/
decryptBatch still call them. Wire format and legacy-fallback semantics
are unchanged, so existing ciphertext continues to decrypt.
Test imports for compression and sync-file-prefix specs now go via
their source files instead of the trimmed barrel.
* fix(sync-providers): bound dropbox token refresh to single retry; share md5 rev helper
The five hand-rolled token-refresh blocks in Dropbox.{getFileRev,
downloadFile, uploadFile, removeFile, listFiles} recursed on themselves
after refresh. If the post-refresh call still saw a token error (real
case: the refresh token itself was revoked), the recursion would not
terminate. Consolidated into a single _withTokenRefresh helper that
attempts the call, refreshes once on a token error, retries once, then
lets the outer 401 classifier surface AuthFailSPError.
Same log message, same _isTokenError discriminator, same refresh call.
Same five sites still apply their post-call non-token error mapping
(NoRev, InvalidData, RemoteFileNotFound, path-not-found swallow, etc.).
Also extracts md5 content-rev computation duplicated between
LocalFileSyncBase._getLocalRev and WebdavApi._computeContentHash into a
shared file-based/content-rev.ts; both call sites preserve their own
error wrapping at the boundary.
* refactor(sync): split oversized super-sync and conflict-resolution
sync-providers: extract request-ID hashing from super-sync.ts (1017 ->
918 lines) into a new request-id.ts. The helpers were free functions
already in disguise (none referenced this), so the move is mechanical.
HTTP plumbing (_doWebFetch/_doNativeFetch/_fetchApi*) stays as private
methods — it transitively touches 12 instance members and would need
either a wide context object or a separate http-client collaborator
class to extract cleanly. Left as a follow-up.
sync-core: split conflict-resolution.ts into three cohesive files:
- entity-frontier.ts now owns buildEntityFrontier and
adjustForClockCorruption (per-entity vector-clock domain).
- extractEntityFromPayload and extractUpdateChanges move to
operation.types.ts next to the existing extractActionPayload.
- conflict-resolution.ts keeps deep-equality, LWW planning,
partitioning, and identical-conflict detection.
Public barrel exports unchanged; tests now import the moved symbols
from their new homes.
* refactor(sync-core): drop redundant OperationStorePort
OperationStorePort overlapped with RemoteOperationApplyStorePort on the
two state-transition methods (markSynced/markApplied,
markRejected/markFailed) and had zero non-structural consumers — the
only implementer was OperationLogStoreService, which already exposes
the three methods as its own public surface. Removing the port leaves
the service contract intact and removes the verb-pair confusion noted
in the post-extraction review.
Spec contract test still drives the same state transitions; only the
local typing of the test fixture changes from the deleted interface to
Pick<OperationLogStoreService, ...>.
* refactor(sync-providers): decouple SuperSync provider from SP-specific host
Two coupling leaks the package shouldn't carry:
1. SUPER_SYNC_DEFAULT_BASE_URL was an implicit fallback inside
SuperSyncProvider — an SP-specific URL baked into a "framework-
agnostic" package. Make defaultBaseUrl a required SuperSyncDeps
field; the host factory supplies the SP default. The constant stays
exported as a suggested default for hosts targeting the SP-hosted
server.
2. Consumers that wanted the WebSocket path had to do
`provider as unknown as SuperSyncProvider` to call
getWebSocketParams. Introduce SuperSyncWebSocketAccess interface +
isSuperSyncWebSocketAccess structural guard; SuperSyncProvider
implements it. sync-wrapper.service drops its cast in favor of the
guard.
super-sync-restore.service still casts to SuperSyncProvider for the
restore path — same pattern would solve it, but out of scope here.
* test(sync-providers): extract shared test helpers and prefer barrels
Adds tests/helpers/sync-logger.ts and tests/helpers/credential-store.ts
to centralize the noopLogger and CredentialStore mocks that were copy-
pasted across 8 spec files. createStatefulCredentialStore covers the
"load/upsert/clear with state" cases; createMockCredentialStore covers
bare vi.fn() ports. Spec sites that needed a unique mockResolvedValue
chain it after the helper, preserving behavior 1:1.
Also migrates 5 spec files from deep ../src/<file> paths to the
matching sub-barrel (../src/webdav, /http, /super-sync, /platform) for
symbols already exported there. No new barrel exports added — internal
types (WebDavHttpAdapter, WebdavApi, DropboxApi, etc.) stay on deep
paths because they are intentionally not part of the public surface.
super-sync.spec.ts keeps its own credential/logger mocks (special
__asPort wrapper and vi.spyOn against the live NOOP_SYNC_LOGGER) that
the generic helpers cannot reproduce without bloat.
* test(sync): pin vector-clock pruning, error-meta privacy, and sync-import edges
Fills three test gaps surfaced by the post-extraction review:
- vector-clock pruning correctness across clocks: 4 cases pinning that
pruning legitimately flips GREATER_THAN to CONCURRENT/LESS_THAN when
the dropped keys are still present in the comparison clock. This is
the documented behavior (compareVectorClocks is intentionally not
pruning-aware); the protocol handles flips server-side via the
rejected-ops retry loop. preserveClientIds case also covered.
- error-meta privacy boundary: 22 new cases covering urlPathOnly (strip
query/fragment/userinfo, preserve host+path+port, leave non-URLs
intact) and errorMeta (no leakage of headers, response bodies, OAuth
tokens, signed-URL params, user emails, or attached error fields).
Real negative assertions (.not.toContain), not shape checks.
- sync-import-filter edge cases: 8 cases covering empty clocks on
either side, op clock listing the import client at 0, same-client
with equal counter (pinning the strict-greater-than boundary), and
different-client knowledge above the import counter.
sync-core 195 -> 207 tests, sync-providers 319 -> 341 tests; no
production code changed.
* style(sync-core): format sync-file-prefix.spec import line
* fix(sync): address package review feedback
tsc -p electron/tsconfig.electron.json was resolving @sp/* aliases to
packages/*/src/*.ts, then transitively compiling those sources and
emitting .js next to each .ts (no outDir is set). Every electron-bearing
task (electron:build, npm start, build, dist) re-littered
packages/{sync-core,sync-providers,shared-schema}/src with stale .js
shadows; gitignore hid them from git but they still shadowed the .ts
sources for Vitest, silently breaking module mocks.
Point the aliases at dist/index.d.ts (and dist/* for subpaths) so tsc
consumes declarations only and never touches package source. Runtime
resolution is unaffected (Node uses the package.json exports).
