Skip to content

Develop - #33

Merged
EarMaster merged 17 commits into
mainfrom
develop
Oct 11, 2026
Merged

EarMaster merged 17 commits into
mainfrom
develop

Conversation

@EarMaster

Copy link
Copy Markdown
Owner

No description provided.

EarMaster and others added 17 commits October 11, 2026 13:59
Stash and context ids arrive from sync verbatim, and download_attachment_from_cloud
joined them straight onto ~/.stashpad/cache. Nothing on the client or the server
checked their shape, so a stash pushed with an id like ..\..\Startup (or an
absolute path) put its attachment wherever it pointed, on every device linked to
the account, with no click needed - the Startup folder turns that into code
execution at the next login. Every other cache path already went through
safe_component; this one did not.

Ids are now checked at the door. A new ids module accepts only a hyphenated UUID
(plus the literal "default" context id), and import_stashes, insert_local_stashes
and import_contexts drop any record that fails it. Attachments are also bound to
the stash they arrive under rather than to whatever stash_id they name. The
download re-checks the ids, for rows written before this, and refuses a target
that canonicalizes to anywhere outside the cache.

Tests that fed made-up ids like "s-remote" through the import path now use UUIDs,
and a new one pushes path-shaped stash, context and attachment ids and asserts
none are written. cargo test, clippy -D warnings and fmt are clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sync upsert kept any non-empty file_path an incoming attachment carried, and
upload_attachment_to_cloud later read that path and PUT the bytes to a URL the
server supplied. The real server never sends a path, so this needed a hostile or
compromised one - but ENCRYPTION.md treats the server as untrusted, and naming
~/.ssh/id_ed25519 or a browser profile would have had the client upload it.

A sync import now records no path at all (a local import still does), and both
places that read a recorded path to send its bytes away - the upload and the
re-encryption sweep - refuse anything that does not canonicalize into
~/.stashpad/cache, via a new utils::is_inside_cache.

A new test imports an attachment carrying /home/user/.ssh/id_ed25519 and asserts
the stored path is empty. cargo test, clippy -D warnings and fmt are clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
safe_entry_path rejected only ParentDir and RootDir components. On Windows an
entry named C:version.dll has a Prefix and no root, so it passed, and base.join
then replaced the base with a path relative to the process's working directory on
that drive. read_import_archive does this during the preview, before the user
confirms anything, so a shared context export could drop a file - a DLL next to
the executable, say - wherever the working directory happened to be.

Entries now pass only if every component is Normal or CurDir.

A Windows-only test covers C:version.dll, an absolute drive path and a UNC path;
the existing traversal test gains an absolute Unix path. cargo test, clippy and
fmt are clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
injectBadges runs after DOMPurify, deliberately: the badge carries an SVG icon and
inline --tag-* variables the sanitizer would strip (d7955aa moved it there for
that reason). But it found text by splitting the sanitized HTML on <[^>]*>, which
assumes no attribute value contains '>'. A link title can, so
[x](https://a "q> #foo <img onerror=...>") split mid-attribute, the inserted badge
closed the title, and the rest of it came out as live elements. The CSP stopped
script, but a form, a <base> or a full-window overlay got through, from content
that can arrive by sync or from an agent writing over MCP.

The step still runs after sanitizing, but now parses the result into a template,
walks text nodes only (skipping pre and code, as before) and replaces each match
with the same badge markup. Text nodes never sit inside a tag, so the class of bug
is gone rather than patched. The app CSP also gains object-src, base-uri,
form-action and frame-src 'none'; the one <form> in the app prevents its default.

The existing tag tests (headings, hex colours, URL anchors, mid-word hashes)
pass unchanged; new ones feed the title payload and assert no img or b element
survives, and that a tag inside link text is still badged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
open_field and open_attachment_metadata let any plaintext value through, because
an account part-way through converting legitimately holds both shapes. Nothing
stopped that after conversion finished, so a malicious or compromised server
could write unsealed content or attachment details into a sealed account and
the client would accept and push them back as genuine - breaking the per-slot
integrity ENCRYPTION.md claims, and making content and id injection easier.

fetch_state now marks the session sealed whenever the server reports the
account's state as sealed, and the flag only goes one way until logout clears
the key. While it is set, a non-empty plaintext field drops the record from
the merge exactly like one that fails to open, and plaintext attachment details
are an error. Empty fields still pass: a sealed account's tombstones carry no text.

The limit: the state still comes from the server, so a server that lies from
the very first request is not caught; one that changes its story is.

A new test seals the session and asserts an injected plaintext stash is dropped,
a contentless one is kept, and plaintext attachment details are refused.
cargo test, clippy and fmt are clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
These all assume a foothold in the webview (the A4 kind); each was a place where
that foothold reached much further than the UI ever needs.

- save_settings restored the stored cloud token whatever endpoint the webview
  sent, so a new endpoint received Bearer <token> on the next sync. The token is
  now restored only when the endpoint is unchanged (ignoring a trailing slash).
- commit_import joined an unchecked token onto the transfer root and later
  remove_dir_all'ed it, so "..\.." deleted the home folder. It now requires the
  UUID read_import_archive issues, as discard_import already did.
- delete_asset compared paths lexically, so cache/../../x passed. A path with
  any .. component is now refused.
- The asset protocol scope was all of ~/.stashpad, including the database,
  settings.json and the key files. It is now cache/ only.
- JS had fs read and write over ~/.stashpad, Downloads, Desktop and Documents but
  only ever calls stat and readFile. The write permissions are gone and the
  ~/.stashpad part of the scope is cache/ plus the browser-drop scratch folder.

Left for later, and noted on the stash: connect-src is still * (AI providers can
be any host), save_asset_from_path still copies any dropped file (that is the
feature), and AI provider calls - and so the AI key - still live in the webview.

cargo test, clippy, fmt and the full vitest suite pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
npm audit flagged DOMPurify 3.3.2 and Svelte 5.45.6. Most advisories concern
modes this app does not use (DOMPurify's IN_PLACE, RETURN_DOM and hook modes,
Svelte's SSR), but Svelte's DOM-clobbering one is relevant because the markdown
sanitizer allowed id and name on rendered content - attributes no markdown output
here needs, and exactly what clobbering relies on. Both are bumped (3.4.16, 5.57.2)
and id/name are removed from ALLOWED_ATTR.

drag was a git dependency with no rev, so a fresh lock could pull whatever its
default branch held. It is pinned to the commit the lockfile already used.

The remaining audit entries are build-time tooling (esbuild, postcss-selector-
parser) whose fixes need breaking major bumps.

vitest, cargo check and vite build pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cloud now answers /sync/* and /attachments/* with a 403 coded
subscription_required for an account without cloud storage, and storage_full
when an upload would exceed the allowance. Every non-success status used to
become a generic "Stash sync failed (403): ..." string, which the sync cycle
would show and then retry identically every few minutes.

coded_refusal turns those two codes into UiErrors (cloud.subscription_required,
cloud.storage_full) at the stash sync, context sync and upload-request call
sites. On subscription_required, cloud-sync.ts re-reads the account so
shouldSync() stops the cycle and Settings shows its existing upgrade option.
Both messages are translated in en, de and zh.

A unit test covers the mapping, including that an uncoded 403 and a coded
non-403 are left alone. cargo test, clippy and the vitest suite pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… use

Third-party actions ran from tags that can be moved - dtolnay/rust-toolchain
from a branch, swatinem/rust-cache@v2, tauri-apps/tauri-action@v1,
codecov-action@v7 - in jobs that hold the Tauri updater key and the Apple
notarization credentials. A moved tag or a compromised maintainer (the
tj-actions/changed-files pattern) would have exfiltrated the updater key, and
with it the ability to sign an update every installed app accepts.

Every action, first-party ones included, is now pinned to a full commit SHA
with its version in a comment, which the existing Dependabot github-actions
config keeps current. dtolnay/rust-toolchain is given `toolchain: stable`
explicitly, since pinned to a SHA it can no longer read it from the ref.

The signing keys still reach $GITHUB_ENV conditionally - an exported empty
value makes the bundler sign with nothing and fail, so they cannot be scoped to
one step with env: - but a new step right after tauri-action blanks them, on
any outcome, so later steps and post-steps (rust-cache's runs last) no longer
see them. tauri-action and the npm build it runs still do; separating the build
from signing is left for later.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The screenshots workflow ran `npm ci` and the capture - the app's code and every
dependency's - and then, in the same job, checked out the website repository with
WEBSITE_SYNC_TOKEN and pushed to its main, which is a production deploy. Anything
the earlier steps left running could have read that token from
website/.git/config. The workflow also had no permissions block, and pasted the
release tag name straight into a shell script, where a tag containing $(...)
would have run.

Capture is now its own job with no secret, a read-only token and no persisted
credentials, and it hands the pictures over as an artifact. A separate publish
job, which runs only git and cp, holds the website token. The tag reaches the
script through the environment. The workflow and test.yml both declare
contents: read rather than inheriting the repository default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The desktop connected to /ws?token=<session>. The cloud stripped queries from its
own logs, but Cloudflare and any proxy on the way could still record the URL, and
with it a long-lived device session.

The upgrade request now carries the token in its Authorization header, which the
cloud's auth middleware already reads; the URL is just /ws. The connect step
returns a plain error string, which is all the reconnect loop ever did with it.
The cloud stops accepting ?token= in the same release.

cargo test, clippy -D warnings and fmt are clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
While the cloud link dialog was open, any stashpad://auth/callback?token=... was
exchanged on the spot. A page the user visited during that window could fire one
with a code for the attacker's account, and the app would link to it and start
syncing the user's stashes there - and, through a synced record, into the
path-traversal territory the id checks now close.

Each time the dialog opens it makes a random nonce and puts it on the account
URL it opens in the browser. The website carries it back on the stashpad://
link, and the app ignores any link without it. Typing the code by hand is
unaffected.

vitest passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…twork

ai-service.svelte.ts called the provider with fetch, so get_settings had to hand
the AI key to the webview, and the app CSP had connect-src * because providers
can be on any host. A script running in the window could therefore read the key
and send it, or anything else, wherever it liked.

The requests now go through two backend commands in a new ai module:
ai_enhance uses the saved endpoint, model and key as they are, and
ai_test_connection tests what the settings form shows, pairing an empty key with
the saved one only when the endpoint is the saved endpoint. get_settings blanks
the key and reports hasApiKey instead, and save_settings keeps a saved key only
for the endpoint it was entered for - otherwise a new endpoint from the window
would inherit it. Settings shows "Saved - type to replace" (en, de, zh) when a
key is stored. With the window making no requests of its own, connect-src is
now just Tauri's IPC; dev builds get a devCsp that also allows Vite's socket.

cargo test, clippy, fmt, vitest and tsc pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The window could read anything in Downloads, Desktop and Documents through the fs plugin,
and save_asset_from_path, read_import_archive and the export commands took whatever path
they were sent. A script in the page could copy any readable file into an attachment and
from there into sync.

The backend now records the paths of drops (from the window event, before the webview
hears of it) and of the file dialogs, which run in Rust instead of the webview. Every
command that touches an outside path checks against that record. Resizing a dropped
image reads it through the backend, the size checks ask the backend, and the fs plugin is
gone from both the frontend and the capabilities.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A link to a completed stash was struck through, which read as if the stash were gone.
It now keeps its checkmark and takes the green of the complete button, as does the
hover preview, and the editor's reference picker marks completed stashes the same way.
Strikethrough is kept for links to deleted stashes. Link labels are also capped at 42
characters instead of 60, so a line of them stays readable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The default find-in-page could not see most of a queue: completed stashes are folded
away and long cards are clamped. Cmd/Ctrl+F now searches the stashes themselves - their
text, AI-enhanced versions and attachment names - forgiving typos and unfinished words.
In one context the queue narrows to the matches, opens clamped cards and marks the
words; pressed again, the search covers every context and lists results grouped by
context, one line per match. Previous and next buttons step through the matches, and a
result from another context takes you there.

Closing the search restores the completed section, and the search waits for a pause in
typing and two letters before it runs, so neither starting nor closing it stalls the
window. Sealed attachment names that have not been downloaded yet are not indexed.

Adds minisearch for the matching.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@EarMaster
EarMaster merged commit 369e3ba into main Oct 11, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant