From ee45aa9726d1adbad56c123bde52341f8b903b44 Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Fri, 28 Aug 2026 15:40:13 -0500
Subject: [PATCH 1/8] A text= click lands on the thing you would have clicked
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
scripts/test-e2e.sh has been exiting 1 on two Suite 2 assertions. Neither was
the app and neither was quite the test.
__resolve matches every element whose trimmed text equals the target, which
for one sidebar row is the
, the inside it, and the inside
that. Click took els[0] — document order — so it clicked the , an element
with no handler. The interaction reported ok and the document never moved.
That is every text= click in the tree, not just this suite's.
It also explains why only one of the two navigation cases failed. Neither
click was working; the second case happened to assert the heading of a file
that was already open, so it passed vacuously. Fixing the targeting is what
makes it a real assertion.
Click now prefers the innermost INTERACTIVE match, falling back to the
innermost match of any kind, whose click still bubbles to whatever handler sits
above it.
The breadcrumb case asserted a control the native app does not have.
web/src/lib/PathBreadcrumb.svelte is imported by nothing — only a stale rule in
the hosted stylesheet mentions it — and a live window reports zero matches for
[class*=breadcrumb], nav[aria-label] and [data-slot*=breadcrumb]. The selector
could only ever find nothing, so the case asserted the absence of a control
rather than any behaviour. It now asserts what it was reaching for, against
something that exists: the sidebar marks the open row data-active="true" and
carries its full path.
Both navigations now wait for the heading to BECOME what they expect. Waiting
for `h1` cannot do that — the previous document's h1 already satisfies it, so
the wait returned instantly and the fixed sleep after it raced the navigation.
Fixes attn-537h.
---
scripts/test-e2e.sh | 79 ++++++++++++++++++++++++++++++---------------
src/daemon.rs | 31 +++++++++++++++++-
2 files changed, 83 insertions(+), 27 deletions(-)
diff --git a/scripts/test-e2e.sh b/scripts/test-e2e.sh
index 4db1029a..e82667a3 100755
--- a/scripts/test-e2e.sh
+++ b/scripts/test-e2e.sh
@@ -77,6 +77,35 @@ assert_truthy() {
fi
}
+# Poll `--eval` until it returns something truthy, or give up after ~4s.
+# Defined up here rather than beside its first use so every suite can reach it.
+poll_eval() {
+ local js="$1"
+ local tries=0
+ local out=""
+ while [ "$tries" -lt 40 ]; do
+ out=$("$ATTN" --eval "$js" 2>/dev/null || echo "")
+ case "$out" in
+ ''|null|'""'|false|0) ;;
+ *) echo "$out"; return 0 ;;
+ esac
+ sleep 0.1
+ tries=$((tries + 1))
+ done
+ echo "$out"
+}
+
+# Wait for the document heading to actually BECOME `$1`.
+#
+# `--wait-for h1` cannot do this: an h1 from the previously-open document
+# already satisfies it, so it returns instantly and whatever fixed sleep
+# follows is racing the navigation. Losing that race made "Navigate to
+# basic.md" read the previous file's heading (attn-537h).
+wait_for_heading() {
+ local expected="$1"
+ poll_eval "(document.querySelector('h1')?.textContent || '').includes('$expected') ? 'yes' : null" >/dev/null
+}
+
screenshot() {
local name="$1"
local path
@@ -233,10 +262,8 @@ echo "--- Navigate Between Files ---"
# Click basic.md in the sidebar
"$ATTN" --click 'text=basic.md'
-# Wait for navigation to complete — h1 should contain "Project Status"
-"$ATTN" --wait-for 'h1' --timeout 5000 >/dev/null 2>&1
-# Give rendering a moment to settle after navigation
-sleep 0.3
+# Wait for the heading to become basic.md's, not merely for AN h1 to exist.
+wait_for_heading "Project Status"
result=$("$ATTN" --query 'h1' | jq -r '.elements[0].text' 2>/dev/null || echo "")
assert_contains "Navigate to basic.md" "$result" "Project Status"
screenshot "05-navigate-basic"
@@ -255,15 +282,31 @@ sleep 0.3
# and the row text contains the filename — `text=` matches text content.
"$ATTN" --click 'text=child.md'
-# Wait for navigation to complete
-"$ATTN" --wait-for 'h1' --timeout 5000 >/dev/null 2>&1
-sleep 0.3
+wait_for_heading "Nested Document"
result=$("$ATTN" --query 'h1' | jq -r '.elements[0].text' 2>/dev/null || echo "")
assert_contains "Navigate to nested child.md" "$result" "Nested Document"
-# Verify breadcrumb shows nested path
-result=$("$ATTN" --query '[class*="breadcrumb"], nav[aria-label]' | jq -r '.elements[0].text // ""' 2>/dev/null || echo "")
-assert_contains "Breadcrumb shows nested path" "$result" "child.md"
+# The nested file is the one the app considers open.
+#
+# This replaces an assertion on a breadcrumb. The native app has no breadcrumb
+# and has not had one for some time: `web/src/lib/PathBreadcrumb.svelte` is
+# imported by nothing, and a live window reports zero matches for
+# `[class*=breadcrumb]`, `nav[aria-label]` and `[data-slot*=breadcrumb]`. The
+# old selector could only ever find nothing, so the case was asserting the
+# absence of a control rather than any behaviour (attn-537h).
+#
+# What it was reaching for — "the app is showing the nested file" — is real and
+# observable: the sidebar marks the open row `data-active="true"` and carries
+# its full path in `data-path`.
+# `--eval` hands back a JSON-encoded string, which escapes the separators —
+# the path arrives as `...\/nested\/child.md`, so a bare `nested/child.md`
+# never appears in it. Strip the escaping rather than assert on the escaped
+# spelling, so the message still shows a readable path when this fails.
+result=$(poll_eval "(() => {
+ const row = document.querySelector('[data-path][data-active=\"true\"]');
+ return row ? row.getAttribute('data-path') : null;
+})()" | tr -d '\\')
+assert_contains "Nested file is the active sidebar row" "$result" "nested/child.md"
screenshot "06-nested-file"
# ===================================================================
@@ -276,22 +319,6 @@ echo "=== Test Suite 3: Relative Images (images.md) ==="
# Poll a synchronous eval until it stops returning `null`/empty. `--eval` hands
# back whatever the expression evaluates to, JSON-encoded, and does NOT await a
# Promise — so waiting has to happen out here, not in the page.
-poll_eval() {
- local js="$1"
- local tries=0
- local out=""
- while [ "$tries" -lt 40 ]; do
- out=$("$ATTN" --eval "$js" 2>/dev/null || echo "")
- case "$out" in
- ''|null|'""'|false|0) ;;
- *) echo "$out"; return 0 ;;
- esac
- sleep 0.1
- tries=$((tries + 1))
- done
- echo "$out"
-}
-
start_daemon "$FIXTURES/images.md"
# Single-file mode, so directory ordering in tests/fixtures/ is irrelevant here.
diff --git a/src/daemon.rs b/src/daemon.rs
index eb089c37..2287e9dc 100644
--- a/src/daemon.rs
+++ b/src/daemon.rs
@@ -1361,6 +1361,35 @@ function __resolve(sel) {
}
return Array.from(document.querySelectorAll(sel));
}
+
+// Which of several `text=` matches should a CLICK land on.
+//
+// A text match is satisfied by every element whose trimmed text equals the
+// target, which for one sidebar row is the , the inside it, and
+// the inside that. Taking the first in document order takes the —
+// an element with no click handler — so the click reported success and nothing
+// happened. That is how "Navigate to basic.md" failed while its sibling case
+// passed vacuously: the document had not moved for either, and the sibling
+// happened to already be open (attn-537h).
+//
+// Prefer the innermost INTERACTIVE match, since that is the thing a person
+// would have clicked. Fall back to the innermost match of any kind, whose
+// click still bubbles to whatever handler is above it.
+function __best(els) {
+ if (els.length < 2) return els[0];
+ var interactive = els.filter(function (el) {
+ return el.matches('button, a, [role=button], input, select, textarea, label, [data-sidebar="menu-button"]');
+ });
+ var pool = interactive.length > 0 ? interactive : els;
+ // Innermost = the one containing no other candidate.
+ for (var i = 0; i < pool.length; i++) {
+ var containsAnother = pool.some(function (other) {
+ return other !== pool[i] && pool[i].contains(other);
+ });
+ if (!containsAnother) return pool[i];
+ }
+ return pool[0];
+}
"#;
match action {
@@ -1371,7 +1400,7 @@ function __resolve(sel) {
{resolve_fn}
var els = __resolve({sel_json});
if (els.length === 0) return JSON.stringify({{status:'not_found',selector:{sel_json}}});
-els[0].click();
+__best(els).click();
return JSON.stringify({{status:'ok'}});
}})()"#,
)
From 8991adb921c11b4864b2f9885b8c41bf27be5038 Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Fri, 28 Aug 2026 15:44:25 -0500
Subject: [PATCH 2/8] A share without a curated file list still carries its
images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
attn-udu8 publishes the images a document references, but only for a share
carrying an explicit selection. Without one, selected_share_wire_path returned
None, publish_asset_snapshot failed for want of a portable name, and every
image was scanned, approved, and then dropped with a log line. The reviewer
saw placeholder cards and no frontend change could have helped — the bytes
were never sent.
That is the CLI path: `attn review share `, and the legacy share(path)
wrapper that passes an empty selection. It stayed invisible because the native
Share dialog always sends a selection, so every live verification of attn-udu8
went through the one path that worked.
A share with no curated list still has a root, and every file it publishes
still needs a portable name. Wire paths now derive from that root — the shared
directory, or a shared file's own parent — with membership of the curated list
authorising the name when there is one, and containment in the root when there
is not. Nothing outside the root becomes nameable either way.
Verified against a live relay: `attn review share tests/fixtures/images.md`
publishes all three assets where it previously published none.
Fixes attn-x2zq.
---
src/review/bootstrap.rs | 107 ++++++++++++++++++++++++++++++++++------
1 file changed, 92 insertions(+), 15 deletions(-)
diff --git a/src/review/bootstrap.rs b/src/review/bootstrap.rs
index 3d29e2e0..5152bc77 100644
--- a/src/review/bootstrap.rs
+++ b/src/review/bootstrap.rs
@@ -4703,27 +4703,46 @@ fn selected_share_wire_path(
let Some(record) = all.get(room_id.as_str()) else {
return Ok(None);
};
- if record.selected_paths.is_empty() {
- return Ok(None);
- }
- let canonical_root = std::path::Path::new(&record.path)
- .canonicalize()
- .map_err(|error| {
- BootstrapError::Store(format!(
- "canonicalize selected share root {}: {error}",
- record.path
- ))
- })?;
+ // A share with no curated file list still has a root, and every file it
+ // publishes still needs a portable name (attn-x2zq).
+ //
+ // Returning None here meant `attn review share ` — and the legacy
+ // share(path) wrapper, which passes an empty selection — produced a share
+ // whose images could not travel at all: publish_asset_snapshot fails
+ // without a wire path, so the bytes were scanned, approved, and then
+ // dropped with only a per-image log line. It was invisible in testing
+ // because the native Share dialog always sends a selection.
+ //
+ // The root is the shared directory, or a shared file's own parent. Files
+ // outside it are still refused, by normalized_relative_share_path below.
+ let record_root = std::path::Path::new(&record.path);
+ let root_for_wire = if record.is_dir {
+ record_root.to_path_buf()
+ } else {
+ record_root
+ .parent()
+ .map(std::path::Path::to_path_buf)
+ .unwrap_or_else(|| record_root.to_path_buf())
+ };
+ let canonical_root = root_for_wire.canonicalize().map_err(|error| {
+ BootstrapError::Store(format!(
+ "canonicalize selected share root {}: {error}",
+ root_for_wire.display()
+ ))
+ })?;
let canonical_path = path.canonicalize().map_err(|error| {
BootstrapError::Store(format!(
"canonicalize shared file {}: {error}",
path.display()
))
})?;
- if !record
- .selected_paths
- .iter()
- .any(|selected| std::path::Path::new(selected) == canonical_path)
+ // With a curated list, membership of it is what authorises a wire path.
+ // Without one, containment in the share root is.
+ if !record.selected_paths.is_empty()
+ && !record
+ .selected_paths
+ .iter()
+ .any(|selected| std::path::Path::new(selected) == canonical_path)
{
return Ok(None);
}
@@ -5727,6 +5746,64 @@ mod tests {
}
}
+ #[test]
+ fn a_share_without_a_curated_list_still_mints_wire_paths() {
+ // attn-x2zq. Returning None here meant `attn review share ` and
+ // the legacy share(path) wrapper published a share whose images could
+ // not travel: publish_asset_snapshot needs a portable name, so the
+ // bytes were scanned, approved, and then dropped. Invisible in testing
+ // because the native Share dialog always sends a selection.
+ let root = TempDir::new().expect("tmp");
+ let store_root = root.path().join("store");
+ std::fs::create_dir_all(&store_root).expect("store dir");
+ let docs = root.path().join("docs");
+ std::fs::create_dir_all(docs.join("nested")).expect("docs dir");
+ std::fs::write(docs.join("notes.md"), b"# hi").expect("doc");
+ std::fs::write(docs.join("chart.svg"), b" ").expect("asset");
+ std::fs::write(docs.join("nested/diagram.png"), b"\x89PNG").expect("nested asset");
+
+ let room: RoomId =
+ serde_json::from_value(serde_json::Value::String("room-no-selection".into())).unwrap();
+
+ // A FOLDER share: the root is the folder itself.
+ record_local_share(&store_root, &room, &docs, true).expect("record dir share");
+ assert_eq!(
+ selected_share_wire_path(&store_root, &room, &docs.join("chart.svg"))
+ .expect("wire path")
+ .as_deref(),
+ Some("chart.svg"),
+ "an asset beside the document gets a portable name"
+ );
+ assert_eq!(
+ selected_share_wire_path(&store_root, &room, &docs.join("nested/diagram.png"))
+ .expect("wire path")
+ .as_deref(),
+ Some("nested/diagram.png"),
+ "and keeps its subdirectory"
+ );
+
+ // A SINGLE-FILE share: the root is that file's own parent, so its
+ // siblings are still nameable.
+ let file_room: RoomId =
+ serde_json::from_value(serde_json::Value::String("room-single-file".into())).unwrap();
+ record_local_share(&store_root, &file_room, &docs.join("notes.md"), false)
+ .expect("record file share");
+ assert_eq!(
+ selected_share_wire_path(&store_root, &file_room, &docs.join("chart.svg"))
+ .expect("wire path")
+ .as_deref(),
+ Some("chart.svg"),
+ "a single-file share roots at the file's directory"
+ );
+
+ // Containment still holds: nothing above the root is nameable.
+ std::fs::write(root.path().join("outside.svg"), b" ").expect("outside");
+ assert!(
+ selected_share_wire_path(&store_root, &room, &root.path().join("outside.svg")).is_err(),
+ "a file outside the share root has no wire path"
+ );
+ }
+
#[test]
fn v3_fragment_without_an_owner_key_still_parses() {
// Backward skew: an invite minted before attn-lb7p has no owner key.
From 38d9a7e6710a98a4188c632416b266e4202b84d8 Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Fri, 28 Aug 2026 15:50:02 -0500
Subject: [PATCH 3/8] Every snapshot has to say who published it, not just the
manifest
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
attn-lb7p made workspace manifests prove authorship. Doing it exposed that the
manifest was the only place in the snapshot path where authorship was checked
at all.
Every other snapshot — the documents a reviewer reads, and the image assets
attn-udu8 added — was accepted on a signature from ANY device in the room
directory, because that is all InboundPipeline establishes. A reviewer holding
a comment- or suggest-tier grant is such a device. Nothing stopped one minting
a SnapshotCreated for an arbitrary fileId and having every other reviewer
render it as the owner's document, or as an image inside it.
Snapshots are structurally owner-published: both republish paths go through
find_room_for_path, which needs a local share record that only the sharing
machine has. A reviewer cannot legitimately publish one, so requiring the
owner's signature refuses exactly the events that had no business existing.
Establishing that first is why this is safe — a wrong tightening here does not
degrade an image, it stops the shared document rendering.
The pinned-key verification is now one helper shared with the manifest path
rather than two copies of the same crypto. Rooms with no pinned key — v2, and
v3 rooms joined before invites carried one — keep behaving exactly as they did:
they have no way to establish authorship and must not lose their documents to a
check they cannot satisfy.
Verified live: a real share still renders all four images with zero hydration
rejections. The two forgery tests are mutation-checked — neutering the check
kills them and nothing else.
Fixes attn-1n67.
---
src/review/manager.rs | 219 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 196 insertions(+), 23 deletions(-)
diff --git a/src/review/manager.rs b/src/review/manager.rs
index fe93bc26..70a4c135 100644
--- a/src/review/manager.rs
+++ b/src/review/manager.rs
@@ -4267,6 +4267,49 @@ fn asset_snapshot_update(
})
}
+/// Whether a room could establish who signed a snapshot.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum OwnerSignature {
+ /// The room pins an owner key and this event verified against it.
+ Verified,
+ /// No key is pinned — a v2 room, or a v3 room joined before invites
+ /// carried one. The caller decides what to do instead.
+ NoPinnedKey,
+}
+
+/// Verify that the room owner signed `event`, against the key pinned at join
+/// (attn-lb7p, generalised by attn-1n67).
+///
+/// A full `verify_event` rather than a `signing_key_id` comparison: replay
+/// re-verifies nothing of its own, so a check that merely trusted what the
+/// live import recorded would evaporate on restart.
+fn verify_snapshot_owner_signature(
+ store: &crate::review::store::ReviewStore,
+ room_id: &RoomId,
+ event: &crate::review::model::ReviewEvent,
+) -> Result {
+ use base64::Engine as _;
+
+ let Some(encoded) = crate::review::bootstrap::load_room_access_v3(store.root(), room_id)
+ .map_err(|err| format!("load room access for snapshot authorship: {err}"))?
+ .and_then(|access| access.owner_public_signing_key)
+ else {
+ return Ok(OwnerSignature::NoPinnedKey);
+ };
+ let bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD
+ .decode(encoded.as_bytes())
+ .map_err(|err| format!("pinned owner key base64url decode: {err}"))?;
+ let bytes: [u8; 32] = bytes
+ .as_slice()
+ .try_into()
+ .map_err(|_| "pinned owner key must decode to 32 bytes".to_string())?;
+ let owner = crate::review::crypto::signing::DeviceVerifyingKey::from_bytes(&bytes)
+ .map_err(|err| format!("pinned owner key: {err}"))?;
+ crate::review::crypto::signing::verify_event(&owner, &event.meta, &event.body, &event.auth)
+ .map_err(|err| format!("not signed by the room owner: {err}"))?;
+ Ok(OwnerSignature::Verified)
+}
+
fn rehydrate_snapshot_event(
store: &crate::review::store::ReviewStore,
room_id: &RoomId,
@@ -4292,6 +4335,34 @@ fn rehydrate_snapshot_event(
return;
}
};
+ // Who published this? (attn-1n67)
+ //
+ // Until now the manifest was the ONLY snapshot whose authorship was ever
+ // checked. Every other one — the documents a reviewer reads, and the image
+ // assets attn-udu8 added — was accepted on a signature from ANY device in
+ // the room directory, because that is all InboundPipeline establishes. A
+ // reviewer holding a comment- or suggest-tier grant is such a device, so
+ // nothing stopped one minting a SnapshotCreated for an arbitrary fileId
+ // and having every other reviewer render it as the owner's document.
+ //
+ // Snapshots are structurally owner-published: both republish paths go
+ // through `find_room_for_path`, which needs a local share record that only
+ // the sharing machine has. So requiring the owner's signature refuses
+ // exactly the events that had no business existing.
+ if plaintext.doc_type != crate::review::model::DocType::WorkspaceManifest {
+ match verify_snapshot_owner_signature(store, room_id, event) {
+ Ok(OwnerSignature::Verified) => {}
+ // No pinned key: a v2 room, or a v3 room joined before invites
+ // carried one. Unchanged from before this check existed — those
+ // rooms have no way to establish authorship and must not lose
+ // their documents over it.
+ Ok(OwnerSignature::NoPinnedKey) => {}
+ Err(err) => {
+ tracing::warn!("snapshot hydration rejected: {err}");
+ return;
+ }
+ }
+ }
if plaintext.doc_type == crate::review::model::DocType::WorkspaceManifest
&& let Err(err) = validate_workspace_manifest_binding(store, room_id, event, &plaintext)
{
@@ -4453,34 +4524,14 @@ fn validate_workspace_manifest_binding(
// whatever the import decided. That matters because replay
// (`replay_room_to_webview`) re-verifies nothing of its own — an identity
// check that only held on the live path would evaporate on restart.
- let pinned_owner_key = crate::review::bootstrap::load_room_access_v3(store.root(), room_id)
- .map_err(|err| format!("load room access for manifest authorship: {err}"))?
- .and_then(|access| access.owner_public_signing_key);
- match pinned_owner_key {
- Some(encoded) => {
- use base64::Engine as _;
- let bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD
- .decode(encoded.as_bytes())
- .map_err(|err| format!("pinned owner key base64url decode: {err}"))?;
- let bytes: [u8; 32] = bytes
- .as_slice()
- .try_into()
- .map_err(|_| "pinned owner key must decode to 32 bytes".to_string())?;
- let owner = crate::review::crypto::signing::DeviceVerifyingKey::from_bytes(&bytes)
- .map_err(|err| format!("pinned owner key: {err}"))?;
- crate::review::crypto::signing::verify_event(
- &owner,
- &manifest_event.meta,
- &manifest_event.body,
- &manifest_event.auth,
- )
- .map_err(|err| format!("workspace manifest was not signed by the room owner: {err}"))?;
+ match verify_snapshot_owner_signature(store, room_id, manifest_event)? {
+ OwnerSignature::Verified => {
// The synthetic FileId is deliberately NOT compared here. It is
// the manifest's name — reviewers match snapshots to entries by
// it — and a name a reviewer cannot compute, and could replay if
// it could, is not a credential.
}
- None => {
+ OwnerSignature::NoPinnedKey => {
// No pinned key: a v2 room, or a v3 room joined before the invite
// carried one. Fall back to the original check, which still works
// for the parties that can satisfy it (owners, v2 joiners) and
@@ -5531,6 +5582,128 @@ mod tests {
)
}
+ /// A store + room holding one persisted markdown snapshot, for the
+ /// authorship cases below. Deliberately NOT bound_manifest_case: that one
+ /// builds a manifest, and the point here is the ORDINARY document path
+ /// that had no authorship check at all (attn-1n67).
+ fn document_snapshot_case() -> (
+ TempDir,
+ ReviewStore,
+ RoomId,
+ crate::review::model::ReviewEvent,
+ ) {
+ use crate::review::crypto::ids::derive_room_id;
+ use crate::review::model::{DocType, SnapshotPlaintext};
+
+ let tmp = TempDir::new().expect("tempdir");
+ let store = ReviewStore::open_at(tmp.path().join("reviews")).expect("open store");
+ let room_id = derive_room_id(&[0x44; 32]);
+ let payload = SnapshotPlaintext {
+ doc_type: DocType::Markdown,
+ content: Some("# Shared\n\nHello.\n".to_string()),
+ anchor_index: None,
+ media_type: None,
+ encoding: None,
+ manifest: None,
+ annotation: None,
+ };
+ let event = persist_snapshot_event(
+ &store,
+ &room_id,
+ "document",
+ dummy_id("CQkJCQkJCQkJCQkJCQkJCQ"),
+ dummy_id("CgoKCgoKCgoKCgoKCgoKCg"),
+ "shared.md",
+ &payload,
+ );
+ assert!(
+ store
+ .append_event(&room_id, &event)
+ .expect("append document event")
+ );
+ (tmp, store, room_id, event)
+ }
+
+ #[test]
+ fn a_document_snapshot_signed_by_the_owner_hydrates() {
+ let (_tmp, store, room_id, mut event) = document_snapshot_case();
+ let owner = signing_key(0x11);
+ pin_owner_key(&store, &room_id, &owner);
+ sign_manifest_as(&mut event, &owner);
+ assert!(
+ hydrated(&store, &room_id, &mut event),
+ "the owner's own document must still render"
+ );
+ }
+
+ #[test]
+ fn a_document_snapshot_signed_by_a_reviewer_is_rejected() {
+ // The hole attn-1n67 closes. Before this, ANY device in the room
+ // directory could mint a SnapshotCreated for an arbitrary fileId and
+ // every other reviewer would render it as the owner's document — a
+ // valid signature from a registered participant was the whole check.
+ let (_tmp, store, room_id, mut event) = document_snapshot_case();
+ let owner = signing_key(0x11);
+ let reviewer = signing_key(0x22);
+ pin_owner_key(&store, &room_id, &owner);
+ sign_manifest_as(&mut event, &reviewer);
+ assert!(
+ !hydrated(&store, &room_id, &mut event),
+ "a document signed by a non-owner must not render"
+ );
+ }
+
+ #[test]
+ fn an_asset_snapshot_signed_by_a_reviewer_is_rejected() {
+ // Same hole, reached through the images attn-udu8 added: a forged
+ // asset would otherwise be rendered inside the owner's document.
+ use crate::review::crypto::ids::derive_room_id;
+ use crate::review::model::{DocType, SnapshotAssetEncoding, SnapshotPlaintext};
+
+ let tmp = TempDir::new().expect("tempdir");
+ let store = ReviewStore::open_at(tmp.path().join("reviews")).expect("open store");
+ let room_id = derive_room_id(&[0x55; 32]);
+ let payload = SnapshotPlaintext {
+ doc_type: DocType::Asset,
+ content: Some("PHN2Zy8-".to_string()),
+ anchor_index: None,
+ media_type: Some("image/svg+xml".to_string()),
+ encoding: Some(SnapshotAssetEncoding::Base64url),
+ manifest: None,
+ annotation: None,
+ };
+ let mut event = persist_snapshot_event(
+ &store,
+ &room_id,
+ "asset",
+ dummy_id("CwsLCwsLCwsLCwsLCwsLCw"),
+ dummy_id("DAwMDAwMDAwMDAwMDAwMDA"),
+ "chart.svg",
+ &payload,
+ );
+ assert!(store.append_event(&room_id, &event).expect("append asset"));
+ let owner = signing_key(0x11);
+ pin_owner_key(&store, &room_id, &owner);
+ sign_manifest_as(&mut event, &signing_key(0x22));
+ assert!(
+ !hydrated(&store, &room_id, &mut event),
+ "a forged asset must not render inside the owner's document"
+ );
+ drop(tmp);
+ }
+
+ #[test]
+ fn a_document_snapshot_in_a_room_with_no_pinned_key_is_unchanged() {
+ // v2 rooms and v3 rooms joined before invites carried an owner key
+ // have no way to establish authorship. They must keep working rather
+ // than lose their documents to a check they cannot satisfy.
+ let (_tmp, store, room_id, mut event) = document_snapshot_case();
+ assert!(
+ hydrated(&store, &room_id, &mut event),
+ "a room with no pinned key must behave as it did before"
+ );
+ }
+
#[test]
fn asset_snapshot_update_forwards_only_authenticated_assets() {
use crate::review::model::{DocType, ReviewEventBody, SnapshotAssetEncoding};
From 4447f632aa073937f22ba8429f567fbe6c753395 Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Tue, 1 Sep 2026 20:58:46 -0500
Subject: [PATCH 4/8] Render document images in hosted shares
---
src/review/assets.rs | 5 +-
src/review/model.rs | 58 +++++-
web/src/BrowserReviewApp.svelte | 13 ++
web/src/hosted/app/EditorShell.svelte | 89 ++++++++
web/src/lib/HtmlViewer.svelte | 12 +-
web/src/lib/review/asset-resolution.test.ts | 16 ++
web/src/lib/review/asset-resolution.ts | 13 +-
.../lib/review/browser-asset-registry.test.ts | 77 +++++++
web/src/lib/review/browser-asset-registry.ts | 196 ++++++++++++++++++
web/src/lib/review/browser-session.test.ts | 6 +-
web/src/lib/review/browser-session.ts | 21 +-
web/src/lib/review/browser-share-owner.ts | 4 +-
.../review/browser-share-production.test.ts | 26 ++-
.../lib/review/browser-share-production.ts | 125 +++++++++--
web/src/lib/review/browser-share-resolver.ts | 8 +-
.../review/browser-workspace-manifest.test.ts | 13 +-
.../lib/review/browser-workspace-manifest.ts | 10 +-
.../review/browser-workspace-sharing.test.ts | 26 +++
.../lib/review/browser-workspace-sharing.ts | 148 +++++++++++--
.../lib/review/document-image-sources.test.ts | 13 ++
web/src/lib/review/document-image-sources.ts | 39 ++++
web/src/lib/review/html-shared-assets.test.ts | 20 ++
web/src/lib/review/html-shared-assets.ts | 34 +++
.../lib/review/shared-image-policy.test.ts | 31 +++
web/src/lib/review/shared-image-policy.ts | 166 +++++++++++++++
25 files changed, 1110 insertions(+), 59 deletions(-)
create mode 100644 web/src/lib/review/browser-asset-registry.test.ts
create mode 100644 web/src/lib/review/browser-asset-registry.ts
create mode 100644 web/src/lib/review/document-image-sources.test.ts
create mode 100644 web/src/lib/review/document-image-sources.ts
create mode 100644 web/src/lib/review/html-shared-assets.test.ts
create mode 100644 web/src/lib/review/html-shared-assets.ts
create mode 100644 web/src/lib/review/shared-image-policy.test.ts
create mode 100644 web/src/lib/review/shared-image-policy.ts
diff --git a/src/review/assets.rs b/src/review/assets.rs
index b57ad5dc..7cf152d4 100644
--- a/src/review/assets.rs
+++ b/src/review/assets.rs
@@ -35,6 +35,7 @@ const IMAGE_EXTENSIONS: &[(&str, &str)] = &[
("jpeg", "image/jpeg"),
("gif", "image/gif"),
("webp", "image/webp"),
+ ("avif", "image/avif"),
("bmp", "image/bmp"),
("ico", "image/x-icon"),
("svg", "image/svg+xml"),
@@ -42,11 +43,11 @@ const IMAGE_EXTENSIONS: &[(&str, &str)] = &[
/// Largest single asset that may be published. Generous for a screenshot or a
/// diagram, small enough that one pathological file cannot dominate a share.
-pub const MAX_ASSET_BYTES: u64 = 8 * 1024 * 1024;
+pub const MAX_ASSET_BYTES: u64 = 3 * 1024 * 1024;
/// Ceiling on everything one document contributes. A snapshot is base64url'd
/// into an encrypted envelope, so the wire cost is ~4/3 of this.
-pub const MAX_TOTAL_BYTES: u64 = 32 * 1024 * 1024;
+pub const MAX_TOTAL_BYTES: u64 = 16 * 1024 * 1024;
/// Ceiling on how many assets one document contributes, so a generated file
/// with a thousand thumbnails cannot stall a share.
diff --git a/src/review/model.rs b/src/review/model.rs
index 8b09886e..196f3c1c 100644
--- a/src/review/model.rs
+++ b/src/review/model.rs
@@ -511,9 +511,25 @@ impl WorkspaceSnapshotManifest {
"workspace manifest entries must not be empty",
));
}
- if self.scope == WorkspaceManifestScope::File && self.entries.len() != 1 {
+ // A file-scoped share contains one primary document plus its declared
+ // image dependencies. The document count, rather than total entries,
+ // remains the scope boundary so reviewers can resolve `./image.png`
+ // without turning a current-file share into a multi-document share.
+ if self.scope == WorkspaceManifestScope::File
+ && self
+ .entries
+ .iter()
+ .filter(|entry| {
+ matches!(
+ entry.kind,
+ WorkspaceManifestEntryKind::Markdown | WorkspaceManifestEntryKind::Html
+ )
+ })
+ .count()
+ != 1
+ {
return Err(SnapshotValidationError::new(
- "file-scoped manifest must contain exactly one entry",
+ "file-scoped manifest must contain exactly one document",
));
}
@@ -1890,6 +1906,44 @@ mod tests {
assert!(payload.validate().is_err());
}
+ #[test]
+ fn file_scoped_manifest_allows_one_document_with_image_dependencies() {
+ let document = WorkspaceManifestEntry {
+ file_id: id("AQEBAQEBAQEBAQEBAQEBAQ"),
+ snapshot_id: id("AgICAgICAgICAgICAgICAg"),
+ path: "notes/readme.md".to_string(),
+ kind: WorkspaceManifestEntryKind::Markdown,
+ media_type: None,
+ byte_length: 7,
+ content_hash: content_hash(b"# note\n"),
+ };
+ let image = WorkspaceManifestEntry {
+ file_id: id("AwMDAwMDAwMDAwMDAwMDAw"),
+ snapshot_id: id("BAQEBAQEBAQEBAQEBAQEBA"),
+ path: "notes/chart.png".to_string(),
+ kind: WorkspaceManifestEntryKind::Asset,
+ media_type: Some("image/png".to_string()),
+ byte_length: 4,
+ content_hash: content_hash(&[0x89, 0x50, 0x4e, 0x47]),
+ };
+ let manifest = WorkspaceSnapshotManifest {
+ v: 1,
+ kind: WorkspaceManifestKind::AttnWorkspaceSnapshot,
+ scope: WorkspaceManifestScope::File,
+ // Canonical UTF-8 path ordering, not declaration ordering.
+ entries: vec![document, image],
+ };
+ assert!(
+ manifest.validate().is_err(),
+ "entries must remain canonically sorted"
+ );
+ let mut manifest = manifest;
+ manifest
+ .entries
+ .sort_by(|left, right| left.path.as_bytes().cmp(right.path.as_bytes()));
+ manifest.validate().unwrap();
+ }
+
#[test]
fn workspace_snapshot_corpus_pins_canonical_bytes_and_valid_payloads() {
let corpus: Value = serde_json::from_str(include_str!(
diff --git a/web/src/BrowserReviewApp.svelte b/web/src/BrowserReviewApp.svelte
index 21f8ec0e..81513a11 100644
--- a/web/src/BrowserReviewApp.svelte
+++ b/web/src/BrowserReviewApp.svelte
@@ -60,6 +60,7 @@
import SelectionToolbar from './lib/SelectionToolbar.svelte';
import { deriveFileEntries, latestRenderableSnapshotId } from './lib/review/file-nav';
import { reviewerStatusPresentation } from './lib/review/reviewer-status-model';
+ import { buildSharedAssetResolver } from './lib/review/asset-resolution';
import { reviewStore } from './lib/review/store.svelte';
import {
applyReviewHoverHighlight,
@@ -643,6 +644,16 @@
? sessionState.snapshotDocType
: 'markdown';
});
+ // The session only puts verified asset metadata in reviewStore; the
+ // tab-local asset registry behind this resolver owns the decrypted bytes and
+ // their Blob URL lifetime. Read dependencies eagerly so a newly activated
+ // manifest rebuilds image NodeViews instead of leaving their fallback cards.
+ const resolveReviewAssetUrl = $derived.by(() => {
+ const snapshots = reviewStore.snapshots;
+ const roomId = sessionState.roomId;
+ const docWirePath = displayedSnapshot?.ownerDisplayPath;
+ return buildSharedAssetResolver(snapshots, roomId, docWirePath);
+ });
$effect(() => {
void sessionState.roomId;
@@ -1695,6 +1706,7 @@
(htmlBridge = bridge)}
@@ -1703,6 +1715,7 @@
();
let assetFolderInput = $state();
let previewUrl = $state(null);
+ // Blob URLs stay only in this tab's live component state. Neither the
+ // document nor the workspace store is rewritten: authored relative srcs
+ // remain the source of truth and a missing/rejected asset keeps the normal
+ // image fallback.
+ let localAssetUrls = $state>({});
+ let ownedAssetUrls = new Map();
+ const resolveLocalAssetUrl = $derived.by(() => {
+ const documentPath = activeEntry?.path;
+ const urls = localAssetUrls;
+ if (!documentPath) return () => null;
+ return (src: string): string | null => {
+ const path = sharedAssetPathFor(documentPath, src);
+ return path === null ? null : urls[path] ?? null;
+ };
+ });
/* The empty-canvas invitation (attn-mkmz.5). A brand-new workspace lands on a
blank untitled.md, and the only standing offer to bring a real document in
@@ -815,6 +833,75 @@
};
});
+ // Resolve local image dependencies through OPFS into short-lived Blob URLs
+ // for the hosted owner surface. This is intentionally separate from the
+ // encrypted-share registry: local images are never staged as share
+ // snapshots, and share viewers never receive an OPFS-derived URL.
+ $effect(() => {
+ const entry = activeEntry;
+ const content = bodyText ?? displayText ?? '';
+ const liveAssets = new Map(workspace.entries.map((candidate) => [candidate.path, candidate]));
+ if (!entry || (entry.kind !== 'markdown' && entry.kind !== 'html')) {
+ for (const url of ownedAssetUrls.values()) URL.revokeObjectURL(url);
+ ownedAssetUrls = new Map();
+ localAssetUrls = {};
+ return;
+ }
+ const sources = entry.kind === 'markdown' ? markdownImageSources(content) : htmlImageSources(content);
+ const paths = new Set();
+ for (const src of sources) {
+ const path = sharedAssetPathFor(entry.path, src);
+ const asset = path === null ? undefined : liveAssets.get(path);
+ if (path !== null && asset?.kind === 'asset' && isSupportedSharedImageMediaType(asset.mediaType)) {
+ paths.add(path);
+ }
+ }
+ let cancelled = false;
+ void (async () => {
+ const next = new Map();
+ const created: string[] = [];
+ try {
+ for (const path of paths) {
+ const retained = ownedAssetUrls.get(path);
+ if (retained) {
+ next.set(path, retained);
+ continue;
+ }
+ const result = await service.readEntryBytes(workspace.id, path);
+ if (!result) continue;
+ const bytes = new Uint8Array(result.bytes);
+ try {
+ const mediaType = result.mediaType;
+ if (!isSupportedSharedImageMediaType(mediaType)) continue;
+ const blob = new Blob([bytes.buffer as ArrayBuffer], { type: mediaType });
+ const url = URL.createObjectURL(blob);
+ created.push(url);
+ next.set(path, url);
+ } finally {
+ bytes.fill(0);
+ }
+ }
+ if (cancelled) {
+ for (const url of created) URL.revokeObjectURL(url);
+ return;
+ }
+ for (const [path, url] of ownedAssetUrls) {
+ if (!next.has(path)) URL.revokeObjectURL(url);
+ }
+ ownedAssetUrls = next;
+ localAssetUrls = Object.fromEntries(next);
+ } catch {
+ for (const url of created) URL.revokeObjectURL(url);
+ }
+ })();
+ return () => { cancelled = true; };
+ });
+
+ $effect(() => () => {
+ for (const url of ownedAssetUrls.values()) URL.revokeObjectURL(url);
+ ownedAssetUrls.clear();
+ });
+
async function createMarkdownFile(): Promise {
const raw = newMarkdownPath.trim();
if (raw.length === 0) return;
@@ -3226,6 +3313,7 @@
onCollabDocChange={handleCollabDocChange}
onCollabSelectionChange={handleCollabSelectionChange}
onCollabViewportChange={handleCollabViewportChange}
+ resolveAssetUrl={resolveLocalAssetUrl}
/>
{/key}
@@ -3242,6 +3330,7 @@
(htmlBridge = bridge)}
diff --git a/web/src/lib/HtmlViewer.svelte b/web/src/lib/HtmlViewer.svelte
index 21c1b265..e518a065 100644
--- a/web/src/lib/HtmlViewer.svelte
+++ b/web/src/lib/HtmlViewer.svelte
@@ -6,6 +6,7 @@
HtmlAnnotationBridge,
injectDocRuntime,
} from './review/html-annotation-bridge';
+ import { rewriteSharedHtmlImageSources } from './review/html-shared-assets';
import type { AnnotationBridgeEvents } from './review/html-annotation-bridge';
interface Props {
@@ -30,6 +31,8 @@
mtime?: number;
/** Native/local pages retain script support; hosted snapshots disable it. */
allowScripts?: boolean;
+ /** Resolves a share-bound HTML image src to an in-memory Blob URL. */
+ resolveAssetUrl?: (src: string) => string | null;
/** Turn on commenting for either a shared source or a local path. */
annotate?: boolean;
/** Wired up once the frame exists, so a parent can drive the rail. */
@@ -43,6 +46,7 @@
content,
mtime,
allowScripts = true,
+ resolveAssetUrl,
annotate = false,
annotationEvents,
onBridge,
@@ -90,9 +94,11 @@
// is why the trust boundary sits in the shell and not in the frame.
// @see planning/collab/html-annotation.md §3, §4
let sandbox = $derived(htmlViewerSandbox(allowScripts || annotating));
- let renderedContent = $derived(
- annotating && content !== undefined ? injectDocRuntime(content) : content,
- );
+ let renderedContent = $derived.by(() => {
+ if (content === undefined) return content;
+ const withSharedAssets = rewriteSharedHtmlImageSources(content, resolveAssetUrl);
+ return annotating ? injectDocRuntime(withSharedAssets) : withSharedAssets;
+ });
let src = $derived.by(() => {
if (isContentMode || path === undefined) return undefined;
const params = new URLSearchParams();
diff --git a/web/src/lib/review/asset-resolution.test.ts b/web/src/lib/review/asset-resolution.test.ts
index e2924ec6..eadd1c49 100644
--- a/web/src/lib/review/asset-resolution.test.ts
+++ b/web/src/lib/review/asset-resolution.test.ts
@@ -4,6 +4,7 @@
// web/scripts/run-tests.mjs, not vitest.
import { sharedAssetPathFor, assetDataUrl, buildSharedAssetResolver } from './asset-resolution';
+import type { BrowserAssetRegistry } from './browser-asset-registry';
import type { ReviewSnapshot } from '../types';
interface CaseResult {
@@ -166,6 +167,21 @@ defineCase('an asset that travelled with the document resolves to its bytes', ()
assertEq(resolve('./diagram.png'), 'data:image/png;base64,AAAA', 'resolved');
});
+defineCase('a verified browser asset resolves through its tab-local Blob URL', () => {
+ const registry = {
+ urlFor: (roomId: string, snapshotId: string) => roomId === 'room-1' && snapshotId === 'snap-diagram.png'
+ ? 'blob:verified-image'
+ : null,
+ } as BrowserAssetRegistry;
+ const resolve = buildSharedAssetResolver(
+ [assetSnapshot('diagram.png', { assetContent: undefined })],
+ 'room-1',
+ 'images.md',
+ registry,
+ );
+ assertEq(resolve('./diagram.png'), 'blob:verified-image', 'verified Blob URL wins over serialized fallback');
+});
+
defineCase('a src with no matching asset resolves to null, not a guess', () => {
// Policy-skipped images (remote, symlink, oversized, budget-exhausted) and
// not-yet-arrived ones are indistinguishable here, and the honest answer for
diff --git a/web/src/lib/review/asset-resolution.ts b/web/src/lib/review/asset-resolution.ts
index 9c0969b4..412dcd16 100644
--- a/web/src/lib/review/asset-resolution.ts
+++ b/web/src/lib/review/asset-resolution.ts
@@ -13,10 +13,12 @@
// strips the share root, admits only `Component::Normal` segments, joins with
// `/`, and NFC-normalises. So they live in one root-relative space and a src
// resolved against the document's own path lands on the asset's key. The
-// manifest is NOT consulted: it is a binding artifact, and the asset snapshot
-// already carries the only key needed.
+// Browser asset bytes are activated only after their snapshot metadata matches
+// the signed manifest, then live as tab-local Blob URLs. Native keeps its
+// existing base64 bridge as a backwards-compatible rendering fallback.
import type { ReviewSnapshot } from '../types';
+import { browserAssetRegistry, type BrowserAssetRegistry } from './browser-asset-registry';
/** `scheme:` — 2+ chars so a `C:` drive letter is not mistaken for one.
* Mirrors `is_non_local` in src/review/assets.rs. */
@@ -125,6 +127,7 @@ export function buildSharedAssetResolver(
snapshots: readonly ReviewSnapshot[],
roomId: string | null,
docWirePath: string | null | undefined,
+ registry: BrowserAssetRegistry = browserAssetRegistry,
): (src: string) => string | null {
if (!roomId || !docWirePath) return () => null;
@@ -151,7 +154,11 @@ export function buildSharedAssetResolver(
if (!snapshot) return null;
const cached = urls.get(snapshot.snapshotId);
if (cached !== undefined) return cached;
- const url = assetDataUrl(snapshot.mediaType, snapshot.assetContent);
+ // Browser sessions intentionally do not put asset payloads on a
+ // ReviewSnapshot. Their hash- and manifest-bound bytes live only in the
+ // tab-local registry; native keeps its existing `assetContent` bridge.
+ const url = registry.urlFor(roomId, snapshot.snapshotId)
+ ?? assetDataUrl(snapshot.mediaType, snapshot.assetContent);
urls.set(snapshot.snapshotId, url);
return url;
};
diff --git a/web/src/lib/review/browser-asset-registry.test.ts b/web/src/lib/review/browser-asset-registry.test.ts
new file mode 100644
index 00000000..5937d81d
--- /dev/null
+++ b/web/src/lib/review/browser-asset-registry.test.ts
@@ -0,0 +1,77 @@
+import { BrowserAssetRegistry, type BrowserObjectUrlApi } from './browser-asset-registry';
+import type { WorkspaceManifestEntry } from '../types';
+
+function assert(value: unknown, message: string): asserts value {
+ if (!value) throw new Error(message);
+}
+
+function assetEntry(overrides: Partial = {}): WorkspaceManifestEntry {
+ return {
+ fileId: 'file-a',
+ snapshotId: 'snapshot-a',
+ path: 'images/chart.png',
+ kind: 'asset',
+ mediaType: 'image/png',
+ byteLength: 24,
+ contentHash: 'hash-a',
+ ...overrides,
+ };
+}
+
+function pngBytes(): Uint8Array {
+ const bytes = new Uint8Array(24);
+ bytes.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
+ bytes.set([0, 0, 0, 2, 0, 0, 0, 2], 16);
+ return bytes;
+}
+
+const revoked: string[] = [];
+let serial = 0;
+const objectUrls: BrowserObjectUrlApi = {
+ createObjectURL: () => `blob:test-${++serial}`,
+ revokeObjectURL: (url) => revoked.push(url),
+};
+const registry = new BrowserAssetRegistry(objectUrls);
+const source = pngBytes();
+
+registry.stage({
+ roomId: 'room-a', fileId: 'file-a', snapshotId: 'snapshot-a', path: 'images/chart.png',
+ mediaType: 'image/png', bytes: source,
+});
+registry.activateManifest('room-a', [assetEntry()]);
+assert(registry.urlFor('room-a', 'snapshot-a') === 'blob:test-1', 'bound asset receives a Blob URL');
+assert(source.every((byte) => byte === 0), 'source bytes are zeroed after the Blob takes a copy');
+
+const unbound = pngBytes();
+registry.stage({
+ roomId: 'room-a', fileId: 'file-b', snapshotId: 'snapshot-b', path: 'images/other.png',
+ mediaType: 'image/png', bytes: unbound,
+});
+registry.activateManifest('room-a', [assetEntry({ snapshotId: 'snapshot-b', path: 'images/forged.png' })]);
+assert(registry.urlFor('room-a', 'snapshot-b') === null, 'a manifest path mismatch cannot mint a Blob URL');
+assert(unbound.some((byte) => byte !== 0), 'unbound data remains pending for a later authentic manifest');
+
+const rejected = new Uint8Array([0, 1, 2, 3]);
+registry.stage({
+ roomId: 'room-a', fileId: 'file-rejected', snapshotId: 'snapshot-rejected', path: 'images/not-an-image',
+ mediaType: 'application/octet-stream', bytes: rejected,
+});
+assert(rejected.every((byte) => byte === 0), 'unsupported payloads are zeroed before they enter the registry');
+assert(registry.urlFor('room-a', 'snapshot-rejected') === null, 'unsupported payloads cannot activate');
+
+const replacement = pngBytes();
+registry.stage({
+ roomId: 'room-a', fileId: 'file-c', snapshotId: 'snapshot-c', path: 'images/chart.png',
+ mediaType: 'image/png', bytes: replacement,
+});
+registry.activateManifest('room-a', [assetEntry({ fileId: 'file-c', snapshotId: 'snapshot-c' })]);
+assert(registry.urlFor('room-a', 'snapshot-a') === null, 'a replacement releases the old snapshot URL');
+assert(registry.urlFor('room-a', 'snapshot-c') === 'blob:test-2', 'a replacement activates the latest verified asset');
+assert(revoked.includes('blob:test-1'), 'replaced URLs are revoked');
+
+registry.clearRoom('room-a');
+assert(registry.urlFor('room-a', 'snapshot-c') === null, 'room teardown clears active URLs');
+assert(revoked.includes('blob:test-2'), 'room teardown revokes active URLs');
+assert(unbound.every((byte) => byte === 0), 'room teardown zeroes pending bytes');
+
+console.log('browser-asset-registry: 10 passed, 0 failed');
diff --git a/web/src/lib/review/browser-asset-registry.ts b/web/src/lib/review/browser-asset-registry.ts
new file mode 100644
index 00000000..b1c96384
--- /dev/null
+++ b/web/src/lib/review/browser-asset-registry.ts
@@ -0,0 +1,196 @@
+// Per-tab runtime for verified shared image bytes.
+//
+// Snapshot plaintext and the Svelte review store deliberately never retain an
+// asset payload. A session stages a hash-verified byte buffer here, then the
+// signed workspace manifest activates the exact matching entry as a Blob URL.
+// That keeps encrypted document bytes out of persisted app state while still
+// giving ProseMirror a synchronous URL resolver.
+
+import type { WorkspaceManifestEntry } from '../types';
+import {
+ bytesMatchSharedImageMediaType,
+ hasSafeSharedImageDimensions,
+ isSupportedSharedImageMediaType,
+ MAX_SHARED_IMAGE_BYTES,
+ MAX_SHARED_IMAGE_COUNT,
+ MAX_SHARED_IMAGE_TOTAL_BYTES,
+} from './shared-image-policy';
+
+export interface VerifiedBrowserAsset {
+ roomId: string;
+ fileId: string;
+ snapshotId: string;
+ path: string;
+ mediaType: string;
+ bytes: Uint8Array;
+}
+
+interface ActiveAsset {
+ roomId: string;
+ snapshotId: string;
+ path: string;
+ url: string;
+ byteLength: number;
+}
+
+export interface BrowserObjectUrlApi {
+ createObjectURL(blob: Blob): string;
+ revokeObjectURL(url: string): void;
+}
+
+function defaultObjectUrls(): BrowserObjectUrlApi | null {
+ if (typeof URL === 'undefined' || typeof Blob === 'undefined') return null;
+ if (typeof URL.createObjectURL !== 'function' || typeof URL.revokeObjectURL !== 'function') return null;
+ return URL;
+}
+
+function snapshotKey(roomId: string, snapshotId: string): string {
+ return `${roomId}\u0000${snapshotId}`;
+}
+
+function pathKey(roomId: string, path: string): string {
+ return `${roomId}\u0000${path}`;
+}
+
+/**
+ * A room-scoped, tab-local registry. Ownership of `bytes` moves to `stage`;
+ * callers must not read or retain that buffer afterwards.
+ */
+export class BrowserAssetRegistry {
+ private readonly pending = new Map();
+ private readonly activeBySnapshot = new Map();
+ private readonly activeByPath = new Map();
+
+ constructor(private readonly objectUrls: BrowserObjectUrlApi | null = defaultObjectUrls()) {}
+
+ stage(asset: VerifiedBrowserAsset): void {
+ const key = snapshotKey(asset.roomId, asset.snapshotId);
+ if (this.activeBySnapshot.has(key)) {
+ // Replay may repeat a snapshot that is already bound and rendered. Keep
+ // the known-good URL rather than allocating a second plaintext Blob.
+ asset.bytes.fill(0);
+ return;
+ }
+ const previous = this.pending.get(key);
+ previous?.bytes.fill(0);
+ this.pending.delete(key);
+ if (!isAllowedAsset(asset) || !this.canStage(asset)) {
+ asset.bytes.fill(0);
+ return;
+ }
+ this.pending.set(key, asset);
+ }
+
+ /**
+ * Activate the assets whose complete metadata exactly matches this signed
+ * manifest. A missing/stale asset remains unavailable instead of becoming a
+ * plausible URL from an unbound snapshot.
+ */
+ activateManifest(roomId: string, entries: readonly WorkspaceManifestEntry[]): void {
+ for (const entry of entries) {
+ if (entry.kind !== 'asset') continue;
+ const key = snapshotKey(roomId, entry.snapshotId);
+ const pending = this.pending.get(key);
+ if (!pending || !matches(entry, pending)) continue;
+ this.pending.delete(key);
+ if (!this.objectUrls) {
+ pending.bytes.fill(0);
+ continue;
+ }
+ const replaced = this.activeByPath.get(pathKey(roomId, entry.path));
+ if (replaced) this.revoke(replaced);
+ let url: string | null = null;
+ let blobBytes: Uint8Array | null = null;
+ try {
+ // Blob's DOM typing requires an ArrayBuffer-backed view; copy out of
+ // a possibly SharedArrayBuffer-backed wire buffer, then zero both
+ // mutable buffers as soon as the Blob has taken its immutable copy.
+ blobBytes = new Uint8Array(pending.bytes);
+ const blob = new Blob([blobBytes.buffer as ArrayBuffer], { type: pending.mediaType });
+ url = this.objectUrls.createObjectURL(blob);
+ const active: ActiveAsset = {
+ roomId,
+ snapshotId: pending.snapshotId,
+ path: pending.path,
+ url,
+ byteLength: pending.bytes.length,
+ };
+ this.activeBySnapshot.set(key, active);
+ this.activeByPath.set(pathKey(roomId, pending.path), active);
+ } finally {
+ blobBytes?.fill(0);
+ pending.bytes.fill(0);
+ // A malformed platform implementation must not leak an inactive URL.
+ if (url !== null && !this.activeBySnapshot.has(key)) this.objectUrls.revokeObjectURL(url);
+ }
+ }
+ }
+
+ urlFor(roomId: string, snapshotId: string): string | null {
+ return this.activeBySnapshot.get(snapshotKey(roomId, snapshotId))?.url ?? null;
+ }
+
+ clearRoom(roomId: string): void {
+ for (const [key, asset] of this.pending) {
+ if (asset.roomId !== roomId) continue;
+ asset.bytes.fill(0);
+ this.pending.delete(key);
+ }
+ for (const asset of [...this.activeBySnapshot.values()]) {
+ if (asset.roomId === roomId) this.revoke(asset);
+ }
+ }
+
+ close(): void {
+ for (const asset of this.pending.values()) asset.bytes.fill(0);
+ this.pending.clear();
+ for (const asset of [...this.activeBySnapshot.values()]) this.revoke(asset);
+ }
+
+ private revoke(asset: ActiveAsset): void {
+ this.activeBySnapshot.delete(snapshotKey(asset.roomId, asset.snapshotId));
+ const key = pathKey(asset.roomId, asset.path);
+ if (this.activeByPath.get(key) === asset) this.activeByPath.delete(key);
+ this.objectUrls?.revokeObjectURL(asset.url);
+ }
+
+ private canStage(asset: VerifiedBrowserAsset): boolean {
+ let count = 1;
+ let bytes = asset.bytes.length;
+ for (const pending of this.pending.values()) {
+ if (pending.roomId !== asset.roomId) continue;
+ count += 1;
+ bytes += pending.bytes.length;
+ }
+ for (const active of this.activeBySnapshot.values()) {
+ if (active.roomId !== asset.roomId) continue;
+ count += 1;
+ bytes += active.byteLength;
+ }
+ return count <= MAX_SHARED_IMAGE_COUNT && bytes <= MAX_SHARED_IMAGE_TOTAL_BYTES;
+ }
+}
+
+function matches(entry: WorkspaceManifestEntry, asset: VerifiedBrowserAsset): boolean {
+ return (
+ entry.fileId === asset.fileId
+ && entry.snapshotId === asset.snapshotId
+ && entry.path === asset.path
+ && entry.mediaType === asset.mediaType
+ && entry.byteLength === asset.bytes.length
+ );
+}
+
+function isAllowedAsset(asset: VerifiedBrowserAsset): boolean {
+ return (
+ isSupportedSharedImageMediaType(asset.mediaType)
+ && asset.bytes.length <= MAX_SHARED_IMAGE_BYTES
+ && bytesMatchSharedImageMediaType(asset.bytes, asset.mediaType)
+ && hasSafeSharedImageDimensions(asset.bytes, asset.mediaType)
+ );
+}
+
+// A module instance belongs to one browser tab. Keeping it here rather than in
+// a Svelte singleton avoids serializing Blob URLs or plaintext through app
+// state, while callers still share one resolver view of a room in that tab.
+export const browserAssetRegistry = new BrowserAssetRegistry();
diff --git a/web/src/lib/review/browser-session.test.ts b/web/src/lib/review/browser-session.test.ts
index 40ef127b..f90a4329 100644
--- a/web/src/lib/review/browser-session.test.ts
+++ b/web/src/lib/review/browser-session.test.ts
@@ -2745,7 +2745,7 @@ defineCase('snapshot parser accepts inert binary/manifest metadata and rejects a
const manifest = parseBrowserSnapshotPlaintext(toCanonicalBytes({
docType: 'workspace_manifest',
manifest: {
- v: 1, kind: 'attn_workspace_snapshot', scope: 'file',
+ v: 1, kind: 'attn_workspace_snapshot', scope: 'entries',
entries: [{
fileId: id(16, 1), snapshotId: id(16, 2), path: 'safe/file.bin', kind: 'asset',
mediaType: 'application/octet-stream', byteLength: 5, contentHash: id(32, 3),
@@ -2839,7 +2839,7 @@ defineCase('manifest waits for an out-of-order recovered R2 entry, then validate
manifest: {
v: 1,
kind: 'attn_workspace_snapshot',
- scope: 'file',
+ scope: 'entries',
entries: [{
fileId: assetFileId,
snapshotId: assetSnapshotId,
@@ -2908,7 +2908,7 @@ defineCase('manifest rejects a hydrated entry whose signed binding differs', asy
const manifest: Extract = {
docType: 'workspace_manifest',
manifest: {
- v: 1, kind: 'attn_workspace_snapshot', scope: 'file', entries: [{
+ v: 1, kind: 'attn_workspace_snapshot', scope: 'entries', entries: [{
fileId: assetFileId, snapshotId: assetSnapshotId, path: 'forged.png', kind: 'asset',
mediaType: 'image/png', byteLength: raw.length, contentHash: contentHash(raw),
}],
diff --git a/web/src/lib/review/browser-session.ts b/web/src/lib/review/browser-session.ts
index 519d661c..25ac485e 100644
--- a/web/src/lib/review/browser-session.ts
+++ b/web/src/lib/review/browser-session.ts
@@ -55,6 +55,7 @@ import {
decodeCanonicalBase64Url,
validateSnapshotPlaintext,
} from './browser-workspace-manifest';
+import { browserAssetRegistry, type BrowserAssetRegistry } from './browser-asset-registry';
import { assembleBrowserEvent, type AssembledBrowserEvent } from './browser-envelope';
import {
BrowserOutbox,
@@ -376,6 +377,8 @@ export interface BrowserSessionOptions {
storage?: BrowserStorage;
/** Override the production IndexedDB opener. */
storageFactory?: (createIfMissing: boolean) => Promise;
+ /** Per-tab verified-asset runtime. Never persisted with the review store. */
+ assetRegistry?: BrowserAssetRegistry;
}
/** Minimal fetch shape — avoids depending on lib.dom.d.ts in TS tests. */
@@ -920,6 +923,7 @@ export function admissionHeaderValue(
*/
export class BrowserSession {
private readonly opts: BrowserSessionOptions;
+ private readonly assetRegistry: BrowserAssetRegistry;
private state: BrowserSessionState = {
principal: 'reviewer',
ownerOnline: false,
@@ -984,6 +988,7 @@ export class BrowserSession {
constructor(opts: BrowserSessionOptions = {}) {
this.opts = opts;
+ this.assetRegistry = opts.assetRegistry ?? browserAssetRegistry;
this.principal = opts.owner ? 'owner' : 'reviewer';
this.state = { ...this.state, principal: this.principal };
this.store = opts.store ?? null;
@@ -1668,6 +1673,7 @@ export class BrowserSession {
/** Tear down transports and clobber in-memory keys. Safe to call repeatedly. */
close(): void {
+ const roomId = this.state.roomId;
this.detachPagehide();
this.stopTransport();
this.reviewInboundDoorbell?.close();
@@ -1680,6 +1686,7 @@ export class BrowserSession {
this.pendingSnapshots.clear();
this.hydratedEntries.clear();
this.pendingWorkspaceManifests.clear();
+ if (roomId) this.assetRegistry.clearRoom(roomId);
this.signerRefreshAttempts.clear();
this.volatileInbound.clear();
this.clearStorePlaintext();
@@ -2206,6 +2213,7 @@ export class BrowserSession {
}
private fail(kind: BrowserSessionError['kind'], message: string): void {
+ const roomId = this.state.roomId;
this.detachPagehide();
this.stopTransport();
this.storage?.close();
@@ -2216,6 +2224,7 @@ export class BrowserSession {
this.pendingSnapshots.clear();
this.hydratedEntries.clear();
this.pendingWorkspaceManifests.clear();
+ if (roomId) this.assetRegistry.clearRoom(roomId);
this.signerRefreshAttempts.clear();
this.volatileInbound.clear();
this.clearStorePlaintext();
@@ -3120,7 +3129,16 @@ export class BrowserSession {
}
snapshot.byteLength = raw.length;
snapshot.mediaType = inline.mediaType;
- raw.fill(0);
+ // Move the verified buffer into the tab-local registry. It stays inert
+ // until the signed workspace manifest binds every field below.
+ this.assetRegistry.stage({
+ roomId: meta.roomId,
+ fileId: body.fileId,
+ snapshotId: body.snapshotId,
+ path: body.ownerDisplayPath ?? '',
+ mediaType: inline.mediaType,
+ bytes: raw,
+ });
} else {
const canonical = toCanonicalBytes(inline.manifest);
if (contentHash(canonical) !== body.baseHash) {
@@ -3144,6 +3162,7 @@ export class BrowserSession {
}
this.pendingWorkspaceManifests.delete(body.snapshotId);
snapshot.workspaceManifest = inline.manifest;
+ this.assetRegistry.activateManifest(meta.roomId, inline.manifest.entries);
}
store.applySnapshot(snapshot);
if (inline.docType !== 'workspace_manifest') {
diff --git a/web/src/lib/review/browser-share-owner.ts b/web/src/lib/review/browser-share-owner.ts
index 19907664..a7738c9e 100644
--- a/web/src/lib/review/browser-share-owner.ts
+++ b/web/src/lib/review/browser-share-owner.ts
@@ -203,8 +203,9 @@ export interface SealDurableSnapshotInput {
epoch: number;
fileId: string;
snapshotId: string;
- docType: 'markdown' | 'html';
+ docType: 'markdown' | 'html' | 'asset';
content: string;
+ mediaType?: string;
metadata?: unknown;
snapshotKey: Uint8Array;
nonce?: Uint8Array;
@@ -233,6 +234,7 @@ export async function sealDurableShareSnapshot(input: SealDurableSnapshotInput):
snapshotId: input.snapshotId,
docType: input.docType,
content: input.content,
+ ...(input.mediaType === undefined ? {} : { mediaType: input.mediaType }),
...(input.metadata === undefined ? {} : { metadata: input.metadata }),
});
let ciphertext: Uint8Array | null = null;
diff --git a/web/src/lib/review/browser-share-production.test.ts b/web/src/lib/review/browser-share-production.test.ts
index f5d0b1eb..f060a706 100644
--- a/web/src/lib/review/browser-share-production.test.ts
+++ b/web/src/lib/review/browser-share-production.test.ts
@@ -1,11 +1,12 @@
-import { base64UrlEncode, deriveShareLinkKeys, expandShareLinkKeys } from './browser-crypto';
+import { base64UrlEncode, contentHash, deriveShareLinkKeys, expandShareLinkKeys } from './browser-crypto';
import { deriveReadKeysV3, toCanonicalBytes } from './browser-crypto';
import { buildShareBundleMutations, EMPTY_SHARE_MANIFEST_DIGEST } from './browser-share-owner';
import { parseAndStripShareInvite } from './browser-share';
import { createBrowserDurableShareResolver, createShareMailboxTransport, decryptDurableShareSnapshot,
DurableShareBrowserSessionFacade, reviewSnapshotFromDurable, subscribeToDurableShareChanges,
- RememberedPushShareSessionFacade,
+ RememberedPushShareSessionFacade, stageDurableAsset,
type BrowserDurableSharePersistence } from './browser-share-production';
+import { browserAssetRegistry } from './browser-asset-registry';
import { indexedDB as fakeIndexedDB } from 'fake-indexeddb';
import { StaleShareEpochError } from './browser-share-session';
import { xchacha20poly1305 } from '@noble/ciphers/chacha.js';
@@ -138,6 +139,27 @@ const inviteUrl = `https://attn.sh/s/${shareId}#key=${base64UrlEncode(secret)}`;
console.log('PASS durable snapshot installation binds resolved room before state patch');
}
+{
+ const raw = new Uint8Array(24);
+ raw.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
+ raw.set([0, 0, 0, 2, 0, 0, 0, 2], 16);
+ const fileId = 'durable-image-file'; const snapshotId = 'durable-image-snapshot';
+ const hash = contentHash(raw);
+ stageDurableAsset({
+ fileId, snapshotId, docType: 'asset', content: base64UrlEncode(raw), mediaType: 'image/png',
+ metadata: { baseHash: hash, manifestEntry: {
+ fileId, snapshotId, path: 'assets/diagram.png', kind: 'asset', mediaType: 'image/png',
+ byteLength: raw.length, contentHash: hash,
+ } },
+ }, 'durable-image-room');
+ assert(browserAssetRegistry.urlFor('durable-image-room', snapshotId)?.startsWith('blob:'),
+ 'offline durable image did not activate a verified Blob URL');
+ const mapped = reviewSnapshotFromDurable({ fileId, snapshotId, docType: 'asset', content: base64UrlEncode(raw), mediaType: 'image/png' }, 'durable-image-room');
+ assert(mapped.assetContent === undefined && mapped.content === undefined, 'durable raw image entered review store');
+ browserAssetRegistry.clearRoom('durable-image-room'); raw.fill(0);
+ console.log('PASS durable image snapshot activates Blob-only renderer state');
+}
+
{
const invite = { shareId, linkSecret: new Uint8Array(secret) };
const persistence = { atomicMax: async ({ candidate }: { candidate: { epoch: number; revision: number; manifestDigest: string } }) => candidate,
diff --git a/web/src/lib/review/browser-share-production.ts b/web/src/lib/review/browser-share-production.ts
index 27def550..0d84cca3 100644
--- a/web/src/lib/review/browser-share-production.ts
+++ b/web/src/lib/review/browser-share-production.ts
@@ -2,6 +2,8 @@ import { sanitizeParticipantColor } from '../participant-color';
import { decompressSnapshotIfNeeded } from './snapshot-compression';
import { boundFetch } from './bound-fetch';
import { compareManifestPathsUtf8 } from './browser-workspace-manifest';
+import { decodeCanonicalBase64Url, isValidSnapshotMediaType } from './browser-workspace-manifest';
+import { browserAssetRegistry } from './browser-asset-registry';
import { xchacha20poly1305 } from '@noble/ciphers/chacha.js';
import { sha256 } from '@noble/hashes/sha2.js';
import {
@@ -62,7 +64,15 @@ import {
replacePushBinding,
type PushBindingRecord,
} from './browser-push-worker';
-import type { Anchor, Capability, ReviewEvent, ReviewEventBody, ReviewSnapshot, SuggestionDraft } from '../types';
+import type {
+ Anchor,
+ Capability,
+ ReviewEvent,
+ ReviewEventBody,
+ ReviewSnapshot,
+ SuggestionDraft,
+ WorkspaceManifestEntry,
+} from '../types';
const DB_NAME = 'attn-browser-durable-shares';
const DB_VERSION = 1;
@@ -448,7 +458,10 @@ export class DurableShareBrowserSessionFacade {
}
close(): void { if (this.closed) return; this.closed = true; ++this.generation; this.startAbort?.abort(); this.startAbort = null;
this.options.invite.linkSecret.fill(0); this.linkSecretForRemember.fill(0);
- this.session?.pushConsent.close(); this.session?.close(); this.session = null; }
+ const roomId = this.state.roomId;
+ this.session?.pushConsent.close(); this.session?.close(); this.session = null;
+ if (roomId) browserAssetRegistry.clearRoom(roomId);
+ }
async createComment(anchor: Anchor, body: string, threadId?: string): Promise {
const event = await this.requireSession().createComment(anchor, body, threadId);
if (!event) throw new Error('comment was queued without an optimistic event');
@@ -530,7 +543,7 @@ export class DurableShareBrowserSessionFacade {
this.observer?.(this.state);
return;
}
- const snapshot = next.snapshots[0];
+ const snapshot = next.snapshots.find((candidate) => candidate.docType !== 'asset');
this.state = { ...this.state,
status: next.status === 'ready' ? 'connected' : next.status === 'error' ? 'error' : 'connecting',
ownerOnline: next.ownerOnline, liveEditingAvailable: false,
@@ -548,6 +561,7 @@ export class DurableShareBrowserSessionFacade {
const { reviewStore } = await import('./store.svelte.js'); reviewStore.applyEvent(event);
}
private async installSnapshot(snapshot: DurableShareSnapshot, roomId: string): Promise {
+ if (snapshot.docType === 'asset') stageDurableAsset(snapshot, roomId);
const value = reviewSnapshotFromDurable(snapshot, roomId);
const { reviewStore } = await import('./store.svelte.js');
reviewStore.currentRoomId = value.roomId; reviewStore.applySnapshot(value);
@@ -555,6 +569,7 @@ export class DurableShareBrowserSessionFacade {
// so unconditionally selecting made the LAST restored file win on every
// reload (and clobbered the URL-requested file). Claim only an empty
// selection; refresh the snapshot pick when the restored file IS selected.
+ if (value.docType === 'asset') return;
if (reviewStore.currentFileId === null) {
reviewStore.setCurrentFile(value.fileId); reviewStore.setCurrentSnapshot(value.snapshotId);
} else if (reviewStore.currentFileId === value.fileId) {
@@ -563,6 +578,51 @@ export class DurableShareBrowserSessionFacade {
}
}
+/** Activate one authenticated durable image only when its sealed metadata
+ * restates an exact manifest entry. The outer ShareDO manifest authenticates
+ * the ciphertext ref; this second check pins the plaintext's path/type/size
+ * before a Blob URL is ever minted. */
+export function stageDurableAsset(snapshot: DurableShareSnapshot, roomId: string): void {
+ const metadata = isRecord(snapshot.metadata) ? snapshot.metadata : null;
+ const rawEntry = metadata?.manifestEntry;
+ if (!isRecord(rawEntry) || !isValidDurableAssetEntry(rawEntry, snapshot)) {
+ throw new Error('durable image snapshot is missing its manifest binding');
+ }
+ const entry = rawEntry as unknown as WorkspaceManifestEntry;
+ const bytes = decodeCanonicalBase64Url(snapshot.content);
+ if (
+ digest(bytes) !== entry.contentHash
+ || metadata?.baseHash !== entry.contentHash
+ || bytes.length !== entry.byteLength
+ ) {
+ bytes.fill(0);
+ throw new Error('durable image snapshot does not match its manifest binding');
+ }
+ browserAssetRegistry.stage({
+ roomId,
+ fileId: snapshot.fileId,
+ snapshotId: snapshot.snapshotId,
+ path: entry.path,
+ mediaType: snapshot.mediaType!,
+ bytes,
+ });
+ browserAssetRegistry.activateManifest(roomId, [entry]);
+}
+
+function isValidDurableAssetEntry(value: Record, snapshot: DurableShareSnapshot): boolean {
+ return (
+ value.fileId === snapshot.fileId
+ && value.snapshotId === snapshot.snapshotId
+ && typeof value.path === 'string'
+ && value.kind === 'asset'
+ && value.mediaType === snapshot.mediaType
+ && isValidSnapshotMediaType(value.mediaType)
+ && Number.isSafeInteger(value.byteLength)
+ && (value.byteLength as number) >= 0
+ && typeof value.contentHash === 'string'
+ );
+}
+
/** Fragmentless notification-click recovery from a locally remembered, non-extractable binding. */
export class RememberedPushShareSessionFacade {
readonly closeOnDestroy = true;
@@ -591,25 +651,33 @@ export class RememberedPushShareSessionFacade {
}
const snapshots = await this.loadSnapshots(binding, abort.signal);
if (this.closed) return;
- const first = snapshots[0];
const store = this.options.store ?? (await import('./store.svelte.js')).reviewStore;
store.currentRoomId = binding.roomId;
- for (const snapshot of snapshots) store.applySnapshot(reviewSnapshotFromDurable(snapshot, binding.roomId));
- if (first && store.currentFileId === null) {
- store.setCurrentFile(first.fileId); store.setCurrentSnapshot(first.snapshotId);
+ for (const snapshot of snapshots) {
+ if (snapshot.docType === 'asset') stageDurableAsset(snapshot, binding.roomId);
+ store.applySnapshot(reviewSnapshotFromDurable(snapshot, binding.roomId));
+ }
+ const firstDocument = snapshots.find((snapshot) => snapshot.docType !== 'asset');
+ if (firstDocument && store.currentFileId === null) {
+ store.setCurrentFile(firstDocument.fileId); store.setCurrentSnapshot(firstDocument.snapshotId);
}
await consumePendingPushEvents(binding.bindingId, event => store.applyEvent(event), {
indexedDB: this.options.indexedDB,
});
if (this.closed) return;
this.patch({ status: 'connected', connection: 'mailbox', roomId: binding.roomId,
- snapshotContent: first?.content ?? null, snapshotDocType: first?.docType ?? 'markdown',
- snapshotId: first?.snapshotId ?? null, fileId: first?.fileId ?? null });
+ snapshotContent: firstDocument?.content ?? null, snapshotDocType: firstDocument?.docType ?? 'markdown',
+ snapshotId: firstDocument?.snapshotId ?? null, fileId: firstDocument?.fileId ?? null });
} catch (error) {
if (!this.closed) this.patch({ status: 'error', error: { kind: 'invite_invalid', message: safeProductionMessage(error) } });
} finally { if (this.abort === abort) this.abort = null; }
}
- close(): void { this.closed = true; this.abort?.abort(); this.abort = null; }
+ close(): void {
+ this.closed = true;
+ this.abort?.abort();
+ this.abort = null;
+ if (this.state.roomId) browserAssetRegistry.clearRoom(this.state.roomId);
+ }
async createComment(): Promise { throw new Error('reopen the original share link to author'); }
async replyToComment(): Promise { throw new Error('reopen the original share link to author'); }
async resolveComment(): Promise { throw new Error('reopen the original share link to author'); }
@@ -673,11 +741,24 @@ export class RememberedPushShareSessionFacade {
export function reviewSnapshotFromDurable(snapshot: DurableShareSnapshot, roomId: string): ReviewSnapshot {
const metadata = isRecord(snapshot.metadata) ? snapshot.metadata : {};
- const baseHash = typeof metadata.baseHash === 'string' ? metadata.baseHash : digest(new TextEncoder().encode(snapshot.content));
+ let byteLength: number;
+ if (snapshot.docType === 'asset') {
+ const bytes = decodeCanonicalBase64Url(snapshot.content);
+ byteLength = bytes.length;
+ bytes.fill(0);
+ } else {
+ byteLength = new TextEncoder().encode(snapshot.content).length;
+ }
+ const baseHash = typeof metadata.baseHash === 'string'
+ ? metadata.baseHash
+ : digest(new TextEncoder().encode(snapshot.content));
return { roomId, fileId: snapshot.fileId, snapshotId: snapshot.snapshotId,
createdAt: Number.isSafeInteger(metadata.createdAt) ? metadata.createdAt as number : Date.now(),
createdBy: typeof metadata.createdBy === 'string' ? metadata.createdBy : 'share-owner', baseHash,
- byteLength: new TextEncoder().encode(snapshot.content).length, docType: snapshot.docType, content: snapshot.content,
+ byteLength, docType: snapshot.docType,
+ ...(snapshot.docType === 'asset'
+ ? { mediaType: snapshot.mediaType }
+ : { content: snapshot.content }),
...(isRecord(metadata.anchorIndex) ? { anchorIndex: metadata.anchorIndex as unknown as ReviewSnapshot['anchorIndex'] } : {}),
...(typeof metadata.ownerDisplayPath === 'string' ? { ownerDisplayPath: metadata.ownerDisplayPath } : {}) };
}
@@ -978,12 +1059,18 @@ export async function decryptDurableShareSnapshot(shareId: string, epoch: number
plaintext = xchacha20poly1305(capability.roomKeys.snapshotKey, sealed.subarray(0, 24), aad).decrypt(sealed.subarray(24));
inflated = await decompressSnapshotIfNeeded(plaintext);
const value = JSON.parse(new TextDecoder().decode(inflated)) as unknown;
- if (!isRecord(value) || Object.keys(value).some(key => !['v','fileId','snapshotId','docType','content','metadata'].includes(key)) ||
+ if (!isRecord(value) || Object.keys(value).some(key => !['v','fileId','snapshotId','docType','content','mediaType','metadata'].includes(key)) ||
value.v !== 3 || value.fileId !== fileId || value.snapshotId !== snapshotId ||
- (value.docType !== 'markdown' && value.docType !== 'html') || typeof value.content !== 'string') {
+ (value.docType !== 'markdown' && value.docType !== 'html' && value.docType !== 'asset') || typeof value.content !== 'string' ||
+ (value.docType === 'asset' && !isValidSnapshotMediaType(value.mediaType))) {
throw new Error('durable share snapshot plaintext is invalid');
}
+ if (value.docType === 'asset') {
+ const bytes = decodeCanonicalBase64Url(value.content);
+ bytes.fill(0);
+ }
return { fileId, snapshotId, docType: value.docType, content: value.content,
+ ...(value.docType === 'asset' ? { mediaType: value.mediaType as string } : {}),
...(value.metadata === undefined ? {} : { metadata: structuredClone(value.metadata) }) };
} finally { aad.fill(0); if (inflated !== plaintext) inflated?.fill(0); plaintext?.fill(0); }
}
@@ -996,7 +1083,7 @@ function disposeBundle(bundle: DecodedDurableShareBundle): void {
function disposeLinkKeys(keys: ShareLinkKeys): void {
keys.linkSecret.fill(0); keys.bundleKey.fill(0); keys.readAdmissionKey.fill(0); keys.writeAdmissionKey?.fill(0);
}
-function disposeSnapshot(snapshot: DurableShareSnapshot): void { snapshot.content = ''; snapshot.metadata = undefined; }
+function disposeSnapshot(snapshot: DurableShareSnapshot): void { snapshot.content = ''; snapshot.metadata = undefined; snapshot.mediaType = undefined; }
function parseRememberedSnapshotRef(value: unknown): { fileId: string; snapshotId: string; ciphertextBytes: number; ciphertextSha256: string; uploadedAt: number } {
if (!isRecord(value) || typeof value.fileId !== 'string' || typeof value.snapshotId !== 'string' ||
!Number.isSafeInteger(value.ciphertextBytes) || (value.ciphertextBytes as number) < 41 ||
@@ -1019,10 +1106,16 @@ async function decryptRememberedSnapshot(shareId: string, epoch: number, _roomId
inflated = await decompressSnapshotIfNeeded(plaintext);
const value = JSON.parse(new TextDecoder().decode(inflated)) as unknown;
if (!isRecord(value) || value.v !== 3 || value.fileId !== fileId || value.snapshotId !== snapshotId ||
- (value.docType !== 'markdown' && value.docType !== 'html') || typeof value.content !== 'string') {
+ (value.docType !== 'markdown' && value.docType !== 'html' && value.docType !== 'asset') || typeof value.content !== 'string' ||
+ (value.docType === 'asset' && !isValidSnapshotMediaType(value.mediaType))) {
throw new Error('remembered snapshot plaintext is invalid');
}
+ if (value.docType === 'asset') {
+ const bytes = decodeCanonicalBase64Url(value.content);
+ bytes.fill(0);
+ }
return { fileId, snapshotId, docType: value.docType, content: value.content,
+ ...(value.docType === 'asset' ? { mediaType: value.mediaType as string } : {}),
...(value.metadata === undefined ? {} : { metadata: structuredClone(value.metadata) }) };
} finally { aad.fill(0); if (inflated !== plaintext) inflated?.fill(0); plaintext?.fill(0); }
}
diff --git a/web/src/lib/review/browser-share-resolver.ts b/web/src/lib/review/browser-share-resolver.ts
index 8f397c30..c12bbf43 100644
--- a/web/src/lib/review/browser-share-resolver.ts
+++ b/web/src/lib/review/browser-share-resolver.ts
@@ -1,7 +1,7 @@
/** Crypto-agnostic durable-share resolution with persistent rollback fencing. */
export type DurableShareTier = 'view' | 'comment' | 'suggest';
-export type DurableShareDocType = 'markdown' | 'html';
+export type DurableShareDocType = 'markdown' | 'html' | 'asset';
const PROTOCOL_ID = /^[A-Za-z0-9_-]{1,128}$/u;
const BUNDLE_ID = /^[A-Za-z0-9_-]{22}$/u;
@@ -53,6 +53,7 @@ export interface DurableShareSnapshot {
snapshotId: string;
docType: DurableShareDocType;
content: string;
+ mediaType?: string;
metadata?: unknown;
}
@@ -330,8 +331,9 @@ export class BrowserShareResolver {
try {
if (
snapshot.fileId !== ref.fileId || snapshot.snapshotId !== ref.snapshotId ||
- (snapshot.docType !== 'markdown' && snapshot.docType !== 'html') ||
- typeof snapshot.content !== 'string'
+ (snapshot.docType !== 'markdown' && snapshot.docType !== 'html' && snapshot.docType !== 'asset') ||
+ typeof snapshot.content !== 'string' ||
+ (snapshot.docType === 'asset' && typeof snapshot.mediaType !== 'string')
) {
throw new BrowserShareResolutionError(
'snapshot_invalid',
diff --git a/web/src/lib/review/browser-workspace-manifest.test.ts b/web/src/lib/review/browser-workspace-manifest.test.ts
index 24b4b7aa..b37b7ae0 100644
--- a/web/src/lib/review/browser-workspace-manifest.test.ts
+++ b/web/src/lib/review/browser-workspace-manifest.test.ts
@@ -83,8 +83,17 @@ rejects(() => validateSnapshotPlaintext({ docType: 'html', content: 'x ',
rejects(() => validateSnapshotPlaintext({ docType: 'markdown', content: 'x', annotation: 'html_selectors_v1' }), 'annotation on markdown');
rejects(() => validateWorkspaceManifest({ ...built, entries: [built.entries[0], built.entries[0]] }), 'duplicate paths/ids');
rejects(() => validateWorkspaceManifest({ ...built, entries: [...built.entries].reverse() }), 'unsorted paths');
-rejects(() => validateWorkspaceManifest({ ...built, scope: 'file' }), 'file scope with many entries');
+equal(
+ validateWorkspaceManifest({ ...built, scope: 'file' }).entries.length,
+ 2,
+ 'file scope permits a document plus its image dependencies',
+);
+rejects(() => validateWorkspaceManifest({
+ ...built,
+ scope: 'file',
+ entries: [...built.entries, { ...markdown, path: 'z/second.md', fileId: base64UrlEncode(new Uint8Array(16).fill(9)), snapshotId: base64UrlEncode(new Uint8Array(16).fill(10)) }],
+}), 'file scope rejects a second document');
rejects(() => validateWorkspaceManifest({ ...built, entries: [{ ...built.entries[0], path: '../raw.bin' }] }), 'escaping path');
rejects(() => validateWorkspaceManifest({ ...built, entries: [{ ...built.entries[0], contentHash: 'AA' }] }), 'short hash');
-console.log(`browser-workspace-manifest: ${vector.cases.length + 14} passed, 0 failed`);
+console.log(`browser-workspace-manifest: ${vector.cases.length + 15} passed, 0 failed`);
diff --git a/web/src/lib/review/browser-workspace-manifest.ts b/web/src/lib/review/browser-workspace-manifest.ts
index 3f27431b..e000c86a 100644
--- a/web/src/lib/review/browser-workspace-manifest.ts
+++ b/web/src/lib/review/browser-workspace-manifest.ts
@@ -61,9 +61,6 @@ export function validateWorkspaceManifest(value: unknown): WorkspaceSnapshotMani
if (!Array.isArray(value.entries) || value.entries.length === 0) {
throw new Error('workspace manifest must contain at least one entry');
}
- if (value.scope === 'file' && value.entries.length !== 1) {
- throw new Error('file-scoped workspace manifest must contain exactly one entry');
- }
const entries: WorkspaceManifestEntry[] = [];
let previousPath: string | undefined;
const fileIds = new Set();
@@ -86,6 +83,13 @@ export function validateWorkspaceManifest(value: unknown): WorkspaceSnapshotMani
previousPath = entry.path;
entries.push(entry);
}
+ if (
+ value.scope === 'file'
+ && (entries.filter((entry) => entry.kind === 'markdown' || entry.kind === 'html').length !== 1
+ || entries.some((entry) => entry.kind === 'asset' && !isValidSnapshotMediaType(entry.mediaType)))
+ ) {
+ throw new Error('file-scoped workspace manifest must contain one document and its asset dependencies');
+ }
return {
v: 1,
kind: 'attn_workspace_snapshot',
diff --git a/web/src/lib/review/browser-workspace-sharing.test.ts b/web/src/lib/review/browser-workspace-sharing.test.ts
index 432b95ac..96954da9 100644
--- a/web/src/lib/review/browser-workspace-sharing.test.ts
+++ b/web/src/lib/review/browser-workspace-sharing.test.ts
@@ -253,6 +253,32 @@ test('publishes one dark ShareDO projection, retained snapshot, then stable tier
} finally { storage.close(); }
});
+test('a current-file share closes over a verified local image and retains it for offline review', async () => {
+ const storage = await openStorage();
+ try {
+ const workspaceId = 'ws-v3-file-image'; await seedWorkspace(storage, workspaceId);
+ const png = new Uint8Array(24);
+ png.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
+ png.set([0, 0, 0, 2, 0, 0, 0, 2], 16);
+ await storage.workspaces.commitRevision({ workspaceId, path: 'assets/image.png', body: png });
+ await storage.workspaces.commitRevision({ workspaceId, path: 'notes/main.md',
+ body: new TextEncoder().encode('# Main\n\n\n') });
+ const fence = await acquireFence(storage, workspaceId); let relay: MemoryShareRelay | null = null;
+ const coordinator = new BrowserWorkspaceSharingCoordinator(storage, workspaceId, fence, {
+ now: () => NOW, randomBytes: deterministicRandom(), createRoom: async options => bootstrapFromOptions(options),
+ publish: options => publishBrowserSnapshots({ ...options, indexBuilder }), indexBuilder,
+ outboxFactory: ({ storage: db, credentials }) => new AckingOutbox(db, credentials.roomId),
+ shareRelayFactory: options => (relay ??= new MemoryShareRelay(options.shareId)),
+ });
+ const view = await coordinator.ensurePublished(request('file', ['notes/main.md']));
+ const rootKey = await storage.getWorkspaceRootKey(workspaceId); assert(rootKey, 'workspace key');
+ const capability = await storage.shares.openShare(rootKey, workspaceId, view.capId);
+ equal(capability.sharePaths, ['assets/image.png', 'notes/main.md'], 'file scope contains its image dependency');
+ assert(required(relay, 'share relay').record?.snapshots.length === 2,
+ 'durable projection retains both document and image');
+ } finally { storage.close(); }
+});
+
test('commits on the first attempt when a deployed relay advances snapshot upload revisions', async () => {
const storage = await openStorage();
try {
diff --git a/web/src/lib/review/browser-workspace-sharing.ts b/web/src/lib/review/browser-workspace-sharing.ts
index 99c01b6c..fc724919 100644
--- a/web/src/lib/review/browser-workspace-sharing.ts
+++ b/web/src/lib/review/browser-workspace-sharing.ts
@@ -74,6 +74,16 @@ import {
type ShareRecordView,
} from './browser-workspace-share';
import { compareManifestPathsUtf8 } from './browser-workspace-manifest';
+import { sharedAssetPathFor } from './asset-resolution';
+import { htmlImageSources, markdownImageSources } from './document-image-sources';
+import {
+ bytesMatchSharedImageMediaType,
+ hasSafeSharedImageDimensions,
+ isSupportedSharedImageMediaType,
+ MAX_SHARED_IMAGE_BYTES,
+ MAX_SHARED_IMAGE_COUNT,
+ MAX_SHARED_IMAGE_TOTAL_BYTES,
+} from './shared-image-policy';
import { normalizeEntryPath, type ShareScopeKind, type WorkspaceEntryRecord } from './browser-workspace-schema';
import type { LeaseHandle } from './browser-workspace-lease';
import type { MailboxEnvelope } from './browser-ws';
@@ -533,15 +543,15 @@ export class BrowserWorkspaceSharingCoordinator {
const sources = await this.loadSources(capability.sharePaths ?? []);
const nextManifest: ManagedShareSnapshotRef[] = [];
try {
- const desired = new Map();
+ const desired = new Map();
for (const entry of manifest.entries) {
const source = sources.find((candidate) => candidate.path === entry.path);
if (!source) throw new StorageConflictError('durable share source path disappeared');
- // The current retained-snapshot plaintext format is text-bearing.
- // Assets remain available through the live ordinary room; a later
- // resolver extension may retain inert binary snapshots as well.
- if (source.docType === 'asset') continue;
- desired.set(entry.fileId, { snapshotId: entry.snapshotId, source });
+ desired.set(entry.fileId, { snapshotId: entry.snapshotId, source, entry });
}
for (const [fileId, wanted] of desired) {
const retained = remote.snapshots.find(
@@ -551,17 +561,35 @@ export class BrowserWorkspaceSharingCoordinator {
nextManifest.push(retained);
continue;
}
- const content = new TextDecoder('utf-8', { fatal: true }).decode(wanted.source.bytes);
- if (wanted.source.docType === 'asset') {
- throw new StorageConflictError('durable text projection selected an asset');
- }
- const metadata = wanted.source.docType === 'markdown'
+ const content = wanted.source.docType === 'asset'
+ ? base64UrlEncode(wanted.source.bytes)
+ : new TextDecoder('utf-8', { fatal: true }).decode(wanted.source.bytes);
+ const anchorIndex = wanted.source.docType === 'markdown'
? await (this.dependencies.indexBuilder
?? (await import('./browser-anchor-index')).buildCanonicalAnchorIndex)(
wanted.source.bytes,
wanted.snapshotId,
)
: undefined;
+ const durableMetadata = {
+ ...(anchorIndex === undefined ? {} : { anchorIndex }),
+ baseHash: wanted.entry.contentHash,
+ ownerDisplayPath: wanted.entry.path,
+ createdAt: this.timestamp(),
+ ...(wanted.source.docType === 'asset'
+ ? {
+ manifestEntry: {
+ fileId: wanted.entry.fileId,
+ snapshotId: wanted.entry.snapshotId,
+ path: wanted.entry.path,
+ kind: 'asset',
+ mediaType: wanted.source.mediaType,
+ byteLength: wanted.source.bytes.length,
+ contentHash: wanted.entry.contentHash,
+ },
+ }
+ : {}),
+ };
const sealed = await sealDurableShareSnapshot({
shareId: credentials.shareId,
epoch: credentials.epoch,
@@ -569,7 +597,8 @@ export class BrowserWorkspaceSharingCoordinator {
snapshotId: wanted.snapshotId,
docType: wanted.source.docType,
content,
- metadata,
+ ...(wanted.source.docType === 'asset' ? { mediaType: wanted.source.mediaType } : {}),
+ metadata: durableMetadata,
snapshotKey: credentials.keys.snapshotKey,
});
try {
@@ -772,31 +801,114 @@ export class BrowserWorkspaceSharingCoordinator {
private async resolvePaths(scopeKind: ShareScopeKind, requested: readonly string[]): Promise {
const entries = await this.storage.workspaces.listEntries(this.workspaceId);
const live = new Map(entries.map((entry) => [entry.path, entry]));
- const paths = scopeKind === 'workspace'
+ const requestedPaths = scopeKind === 'workspace'
? entries.map((entry) => entry.path)
: requested.map((path) => normalizeEntryPath(path));
- if (scopeKind === 'file' && paths.length !== 1) {
+ if (scopeKind === 'file' && requestedPaths.length !== 1) {
throw new BrowserStorageError('current-file share requires exactly one path');
}
- if (paths.length === 0) throw new BrowserStorageError('share scope cannot be empty');
- if (new Set(paths).size !== paths.length) throw new BrowserStorageError('share scope contains duplicate paths');
- for (const path of paths) if (!live.has(path)) throw new StorageConflictError('share scope contains a stale path');
- if (!paths.some((path) => {
+ if (requestedPaths.length === 0) throw new BrowserStorageError('share scope cannot be empty');
+ if (new Set(requestedPaths).size !== requestedPaths.length) throw new BrowserStorageError('share scope contains duplicate paths');
+ for (const path of requestedPaths) if (!live.has(path)) throw new StorageConflictError('share scope contains a stale path');
+ if (!requestedPaths.some((path) => {
const entry = live.get(path);
return entry?.kind === 'markdown' || (entry !== undefined && entryIsHtml(entry));
})) {
throw new BrowserStorageError('share scope must contain at least one Markdown or HTML document');
}
+ // A document scope is a dependency closure, not a hand-maintained file
+ // list. Only local, supported image assets that its document actually
+ // names are added; remote/missing/non-image srcs remain honest fallbacks.
+ const paths = new Set(requestedPaths);
+ const documentPaths = requestedPaths.filter((path) => {
+ const entry = live.get(path);
+ return entry?.kind === 'markdown' || (entry !== undefined && entryIsHtml(entry));
+ });
+ const images = await this.referencedImages(documentPaths, live);
+ for (const image of images) paths.add(image);
return [...paths].sort(compareManifestPathsUtf8);
}
+ private async referencedImages(
+ documentPaths: readonly string[],
+ live: ReadonlyMap,
+ ): Promise {
+ if (documentPaths.length === 0) return [];
+ const candidates = new Set();
+ let totalBytes = 0;
+ for (const documentPath of documentPaths) {
+ const entry = live.get(documentPath);
+ if (!entry) continue;
+ const body = await this.storage.workspaces.getRevisionBody(
+ this.workspaceId,
+ documentPath,
+ entry.headRevisionId,
+ );
+ let sources: string[];
+ try {
+ const text = new TextDecoder('utf-8', { fatal: true }).decode(body);
+ sources = entry.kind === 'markdown' ? markdownImageSources(text) : htmlImageSources(text);
+ } catch {
+ sources = [];
+ } finally {
+ body.fill(0);
+ }
+ for (const src of sources) {
+ const path = sharedAssetPathFor(documentPath, src);
+ if (path === null || candidates.has(path)) continue;
+ const asset = live.get(path);
+ if (!asset || asset.kind !== 'asset' || !isSupportedSharedImageMediaType(asset.mediaType)) continue;
+ candidates.add(path);
+ }
+ }
+ const paths: string[] = [];
+ for (const path of candidates) {
+ if (paths.length >= MAX_SHARED_IMAGE_COUNT) break;
+ const entry = live.get(path);
+ if (!entry || entry.kind !== 'asset' || !entry.mediaType) continue;
+ const bytes = await this.storage.workspaces.getRevisionBody(this.workspaceId, path, entry.headRevisionId);
+ try {
+ if (
+ bytes.length > MAX_SHARED_IMAGE_BYTES
+ || totalBytes + bytes.length > MAX_SHARED_IMAGE_TOTAL_BYTES
+ || !bytesMatchSharedImageMediaType(bytes, entry.mediaType)
+ || !hasSafeSharedImageDimensions(bytes, entry.mediaType)
+ ) continue;
+ paths.push(path);
+ totalBytes += bytes.length;
+ } finally {
+ bytes.fill(0);
+ }
+ }
+ return paths;
+ }
+
private async loadSources(paths: readonly string[]): Promise {
const sources: BrowserSnapshotEntry[] = [];
+ let sharedImageCount = 0;
+ let sharedImageBytes = 0;
try {
for (const path of paths) {
const entry = await this.storage.workspaces.getEntry(this.workspaceId, path);
if (!entry) throw new StorageConflictError('share scope changed before publication');
const bytes = await this.storage.workspaces.getRevisionBody(this.workspaceId, path, entry.headRevisionId);
+ if (entry.kind === 'asset' && isSupportedSharedImageMediaType(entry.mediaType)) {
+ if (
+ bytes.length > MAX_SHARED_IMAGE_BYTES
+ || sharedImageCount >= MAX_SHARED_IMAGE_COUNT
+ || sharedImageBytes + bytes.length > MAX_SHARED_IMAGE_TOTAL_BYTES
+ || !bytesMatchSharedImageMediaType(bytes, entry.mediaType!)
+ || !hasSafeSharedImageDimensions(bytes, entry.mediaType!)
+ ) {
+ // A stale or deliberately oversized local image must not make the
+ // whole document unshareable. Leave its authored src untouched;
+ // the reviewer gets the normal unavailable-image fallback instead.
+ bytes.fill(0);
+ continue;
+ }
+ sharedImageCount += 1;
+ sharedImageBytes += bytes.length;
+ }
sources.push(entry.kind === 'markdown'
? { path, docType: 'markdown', bytes, revisionId: entry.headRevisionId }
: entryIsHtml(entry)
diff --git a/web/src/lib/review/document-image-sources.test.ts b/web/src/lib/review/document-image-sources.test.ts
new file mode 100644
index 00000000..2ceb966b
--- /dev/null
+++ b/web/src/lib/review/document-image-sources.test.ts
@@ -0,0 +1,13 @@
+import { markdownImageSources, srcsetSources } from './document-image-sources';
+
+function equal(actual: readonly string[], expected: readonly string[], message: string): void {
+ if (JSON.stringify(actual) !== JSON.stringify(expected)) {
+ throw new Error(`${message}: expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`);
+ }
+}
+
+equal(markdownImageSources('\n'), ['assets/chart.png'], 'Markdown dependencies are unique');
+equal(markdownImageSources('remote '), ['https://example.test/chart.png'], 'discovery leaves policy to its caller');
+equal(srcsetSources('./chart.png 1x, ./chart@2x.png 2x'), ['./chart.png', './chart@2x.png'], 'srcset candidates retain URL tokens');
+
+console.log('document-image-sources: 3 passed, 0 failed');
diff --git a/web/src/lib/review/document-image-sources.ts b/web/src/lib/review/document-image-sources.ts
new file mode 100644
index 00000000..32693703
--- /dev/null
+++ b/web/src/lib/review/document-image-sources.ts
@@ -0,0 +1,39 @@
+import { markdownParser } from '../schema';
+
+/** Authored image URLs only; callers decide whether each may cross a boundary. */
+export function markdownImageSources(content: string): string[] {
+ const sources = new Set();
+ try {
+ markdownParser.parse(content).descendants((node) => {
+ if (node.type.name === 'image' && typeof node.attrs.src === 'string') sources.add(node.attrs.src);
+ });
+ } catch {
+ // An invalid draft must not block editing or publishing the document; it
+ // simply has no discoverable local-image dependencies until it parses.
+ }
+ return [...sources];
+}
+
+/** HTML snapshots use the same relative-path rules as Markdown images. */
+export function htmlImageSources(content: string): string[] {
+ if (typeof DOMParser === 'undefined') return [];
+ const sources = new Set();
+ const document = new DOMParser().parseFromString(content, 'text/html');
+ for (const image of document.querySelectorAll('img[src]')) {
+ const src = image.getAttribute('src');
+ if (src) sources.add(src);
+ }
+ for (const source of document.querySelectorAll('img[srcset], source[srcset]')) {
+ const srcset = source.getAttribute('srcset');
+ if (!srcset) continue;
+ for (const src of srcsetSources(srcset)) sources.add(src);
+ }
+ return [...sources];
+}
+
+export function srcsetSources(srcset: string): string[] {
+ return srcset.split(',').flatMap((candidate) => {
+ const match = /^\s*(\S+)/u.exec(candidate);
+ return match ? [match[1]!] : [];
+ });
+}
diff --git a/web/src/lib/review/html-shared-assets.test.ts b/web/src/lib/review/html-shared-assets.test.ts
new file mode 100644
index 00000000..cac60fca
--- /dev/null
+++ b/web/src/lib/review/html-shared-assets.test.ts
@@ -0,0 +1,20 @@
+import { rewriteSrcset } from './html-shared-assets';
+
+function assertEqual(actual: string, expected: string, message: string): void {
+ if (actual !== expected) throw new Error(`${message}: expected ${expected}, got ${actual}`);
+}
+
+const resolve = (src: string): string | null => src === './chart.png' ? 'blob:verified-chart' : null;
+
+assertEqual(
+ rewriteSrcset('./chart.png 1x, remote.png 2x', resolve),
+ 'blob:verified-chart 1x, remote.png 2x',
+ 'srcset replaces only a verified local candidate',
+);
+assertEqual(
+ rewriteSrcset(' data:image/png;base64,AAAA 1x', resolve),
+ ' data:image/png;base64,AAAA 1x',
+ 'unresolved data source stays byte-for-byte intact',
+);
+
+console.log('html-shared-assets: 2 passed, 0 failed');
diff --git a/web/src/lib/review/html-shared-assets.ts b/web/src/lib/review/html-shared-assets.ts
new file mode 100644
index 00000000..13d2aac2
--- /dev/null
+++ b/web/src/lib/review/html-shared-assets.ts
@@ -0,0 +1,34 @@
+/**
+ * Rewrites only share-bound HTML image URLs. The source remains inside a
+ * sandboxed opaque-origin iframe; remote, data, and absent local references
+ * remain untouched so no new network capability is introduced here.
+ */
+export function rewriteSharedHtmlImageSources(
+ content: string,
+ resolveAssetUrl: ((src: string) => string | null) | undefined,
+): string {
+ if (!resolveAssetUrl || typeof DOMParser === 'undefined') return content;
+ const document = new DOMParser().parseFromString(content, 'text/html');
+ for (const image of document.querySelectorAll('img[src]')) {
+ const resolved = resolveAssetUrl(image.getAttribute('src') ?? '');
+ if (resolved !== null) image.setAttribute('src', resolved);
+ }
+ for (const source of document.querySelectorAll('img[srcset], source[srcset]')) {
+ const srcset = source.getAttribute('srcset');
+ if (srcset !== null) source.setAttribute('srcset', rewriteSrcset(srcset, resolveAssetUrl));
+ }
+ return `\n${document.documentElement.outerHTML}`;
+}
+
+/** Keep each candidate descriptor intact while replacing its URL token. */
+export function rewriteSrcset(
+ srcset: string,
+ resolveAssetUrl: (src: string) => string | null,
+): string {
+ return srcset.split(',').map((candidate) => {
+ const match = /^(\s*)(\S+)([\s\S]*)$/u.exec(candidate);
+ if (!match) return candidate;
+ const resolved = resolveAssetUrl(match[2]!);
+ return resolved === null ? candidate : `${match[1]}${resolved}${match[3]}`;
+ }).join(',');
+}
diff --git a/web/src/lib/review/shared-image-policy.test.ts b/web/src/lib/review/shared-image-policy.test.ts
new file mode 100644
index 00000000..432a666a
--- /dev/null
+++ b/web/src/lib/review/shared-image-policy.test.ts
@@ -0,0 +1,31 @@
+import {
+ bytesMatchSharedImageMediaType,
+ hasSafeSharedImageDimensions,
+ isSupportedSharedImageMediaType,
+ MAX_SHARED_IMAGE_BYTES,
+ MAX_SHARED_IMAGE_COUNT,
+ MAX_SHARED_IMAGE_TOTAL_BYTES,
+} from './shared-image-policy';
+
+function assert(value: unknown, message: string): asserts value {
+ if (!value) throw new Error(message);
+}
+
+assert(isSupportedSharedImageMediaType('image/png'), 'PNG is allowlisted');
+assert(isSupportedSharedImageMediaType('IMAGE/AVIF'), 'media type comparison is case-insensitive');
+assert(!isSupportedSharedImageMediaType('image/tiff'), 'unsupported image types cannot enter a share');
+assert(!isSupportedSharedImageMediaType('text/html'), 'document media cannot enter a share as an image');
+assert(bytesMatchSharedImageMediaType(new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), 'image/png'), 'PNG signature is checked');
+assert(!bytesMatchSharedImageMediaType(new Uint8Array([0, 1, 2, 3]), 'image/png'), 'mismatched bytes are rejected');
+assert(bytesMatchSharedImageMediaType(new TextEncoder().encode(' '), 'image/svg+xml'), 'SVG accepts leading whitespace');
+const png = new Uint8Array(24);
+png.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
+png.set([0, 0, 0, 100, 0, 0, 0, 200], 16);
+assert(hasSafeSharedImageDimensions(png, 'image/png'), 'bounded PNG dimensions are accepted');
+png.set([0, 0, 0x27, 0x10, 0, 0, 0x27, 0x10], 16);
+assert(!hasSafeSharedImageDimensions(png, 'image/png'), 'oversized raster dimensions are refused before decoding');
+assert(!hasSafeSharedImageDimensions(new Uint8Array(8), 'image/png'), 'unknown raster dimensions fail closed');
+assert(MAX_SHARED_IMAGE_BYTES === 3 * 1024 * 1024, 'single-image cap preserves encrypted-envelope headroom');
+assert(MAX_SHARED_IMAGE_COUNT === 64 && MAX_SHARED_IMAGE_TOTAL_BYTES === 16 * 1024 * 1024, 'aggregate image budgets are bounded');
+
+console.log('shared-image-policy: 12 passed, 0 failed');
diff --git a/web/src/lib/review/shared-image-policy.ts b/web/src/lib/review/shared-image-policy.ts
new file mode 100644
index 00000000..42a5f940
--- /dev/null
+++ b/web/src/lib/review/shared-image-policy.ts
@@ -0,0 +1,166 @@
+// Shared-image policy kept deliberately below the encrypted transport limits.
+// Assets are represented as canonical base64url inside snapshot JSON, so the
+// 5 MiB relay ciphertext ceiling cannot safely admit an 8 MiB raw image.
+
+export const MAX_SHARED_IMAGE_BYTES = 3 * 1024 * 1024;
+export const MAX_SHARED_IMAGE_COUNT = 64;
+export const MAX_SHARED_IMAGE_TOTAL_BYTES = 16 * 1024 * 1024;
+export const MAX_SHARED_IMAGE_PIXELS = 40_000_000;
+
+const IMAGE_MEDIA_TYPES = new Set([
+ 'image/png',
+ 'image/jpeg',
+ 'image/gif',
+ 'image/webp',
+ 'image/avif',
+ 'image/bmp',
+ 'image/x-icon',
+ 'image/svg+xml',
+]);
+
+export function isSupportedSharedImageMediaType(mediaType: string | undefined): boolean {
+ return mediaType !== undefined && IMAGE_MEDIA_TYPES.has(mediaType.toLowerCase());
+}
+
+/** Lightweight content checks before encrypted publication. Rendering remains
+ * in an image context; SVG never receives a script-capable document context. */
+export function bytesMatchSharedImageMediaType(bytes: Uint8Array, mediaType: string): boolean {
+ switch (mediaType.toLowerCase()) {
+ case 'image/png':
+ return startsWith(bytes, [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
+ case 'image/jpeg':
+ return startsWith(bytes, [0xff, 0xd8, 0xff]);
+ case 'image/gif':
+ return startsWith(bytes, text('GIF87a')) || startsWith(bytes, text('GIF89a'));
+ case 'image/webp':
+ return startsWith(bytes, text('RIFF')) && startsWith(bytes.subarray(8), text('WEBP'));
+ case 'image/avif':
+ return bytes.length >= 12 && startsWith(bytes.subarray(4), text('ftyp'))
+ && ['avif', 'avis'].includes(new TextDecoder().decode(bytes.subarray(8, 12)));
+ case 'image/bmp':
+ return startsWith(bytes, text('BM'));
+ case 'image/x-icon':
+ return startsWith(bytes, [0, 0, 1, 0]);
+ case 'image/svg+xml':
+ return new TextDecoder('utf-8', { fatal: false }).decode(bytes.subarray(0, 1024))
+ .replace(/^\uFEFF?\s*/u, '').startsWith(' 0
+ && dimensions[1] > 0
+ && dimensions[0] * dimensions[1] <= MAX_SHARED_IMAGE_PIXELS;
+}
+
+function rasterDimensions(bytes: Uint8Array, mediaType: string): [number, number] | null {
+ switch (mediaType) {
+ case 'image/png':
+ return bytes.length >= 24 ? [u32be(bytes, 16), u32be(bytes, 20)] : null;
+ case 'image/gif':
+ return bytes.length >= 10 ? [u16le(bytes, 6), u16le(bytes, 8)] : null;
+ case 'image/jpeg':
+ return jpegDimensions(bytes);
+ case 'image/webp':
+ return webpDimensions(bytes);
+ case 'image/avif':
+ return ispeDimensions(bytes);
+ case 'image/bmp':
+ return bytes.length >= 26 ? [i32le(bytes, 18), Math.abs(i32le(bytes, 22))] : null;
+ case 'image/x-icon':
+ return bytes.length >= 8
+ ? [bytes[6] === 0 ? 256 : bytes[6]!, bytes[7] === 0 ? 256 : bytes[7]!]
+ : null;
+ default:
+ return null;
+ }
+}
+
+function jpegDimensions(bytes: Uint8Array): [number, number] | null {
+ let offset = 2;
+ while (offset + 9 < bytes.length) {
+ if (bytes[offset] !== 0xff) return null;
+ while (bytes[offset] === 0xff) offset += 1;
+ const marker = bytes[offset++]!;
+ if (marker === 0xd8 || marker === 0xd9) continue;
+ if (offset + 1 >= bytes.length) return null;
+ const length = u16be(bytes, offset);
+ if (length < 2 || offset + length > bytes.length) return null;
+ // Start-of-frame markers except the non-frame DHT/DAC/JPG markers.
+ if (marker >= 0xc0 && marker <= 0xcf && ![0xc4, 0xc8, 0xcc].includes(marker)) {
+ return offset + 7 < bytes.length ? [u16be(bytes, offset + 5), u16be(bytes, offset + 3)] : null;
+ }
+ offset += length;
+ }
+ return null;
+}
+
+function webpDimensions(bytes: Uint8Array): [number, number] | null {
+ if (bytes.length < 30) return null;
+ const chunk = decodeAscii(bytes.subarray(12, 16));
+ if (chunk === 'VP8X') {
+ return [u24le(bytes, 24) + 1, u24le(bytes, 27) + 1];
+ }
+ if (chunk === 'VP8 ') {
+ return [u16le(bytes, 26) & 0x3fff, u16le(bytes, 28) & 0x3fff];
+ }
+ if (chunk === 'VP8L' && bytes.length >= 25 && bytes[20] === 0x2f) {
+ const packed = bytes[21]! | (bytes[22]! << 8) | (bytes[23]! << 16) | (bytes[24]! << 24);
+ return [(packed & 0x3fff) + 1, ((packed >>> 14) & 0x3fff) + 1];
+ }
+ return null;
+}
+
+function ispeDimensions(bytes: Uint8Array): [number, number] | null {
+ // `ispe` carries a FullBox (version+flags) followed by width and height.
+ for (let index = 4; index + 16 <= bytes.length && index < 64 * 1024; index += 1) {
+ if (decodeAscii(bytes.subarray(index, index + 4)) === 'ispe') {
+ return [u32be(bytes, index + 8), u32be(bytes, index + 12)];
+ }
+ }
+ return null;
+}
+
+function u16be(bytes: Uint8Array, offset: number): number {
+ return (bytes[offset]! << 8) | bytes[offset + 1]!;
+}
+
+function u16le(bytes: Uint8Array, offset: number): number {
+ return bytes[offset]! | (bytes[offset + 1]! << 8);
+}
+
+function u24le(bytes: Uint8Array, offset: number): number {
+ return bytes[offset]! | (bytes[offset + 1]! << 8) | (bytes[offset + 2]! << 16);
+}
+
+function i32le(bytes: Uint8Array, offset: number): number {
+ return (bytes[offset]! | (bytes[offset + 1]! << 8) | (bytes[offset + 2]! << 16) | (bytes[offset + 3]! << 24));
+}
+
+function u32be(bytes: Uint8Array, offset: number): number {
+ return ((bytes[offset]! * 0x1_0000_00) + (bytes[offset + 1]! << 16) + (bytes[offset + 2]! << 8) + bytes[offset + 3]!) >>> 0;
+}
+
+function decodeAscii(bytes: Uint8Array): string {
+ return String.fromCharCode(...bytes);
+}
+
+function startsWith(bytes: Uint8Array, prefix: ArrayLike): boolean {
+ if (bytes.length < prefix.length) return false;
+ for (let index = 0; index < prefix.length; index += 1) {
+ if (bytes[index] !== prefix[index]) return false;
+ }
+ return true;
+}
+
+function text(value: string): Uint8Array {
+ return new TextEncoder().encode(value);
+}
From 055e73f4df25514ffd04beee1a2a3d900d1c5546 Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Tue, 1 Sep 2026 21:43:15 -0500
Subject: [PATCH 5/8] Harden hosted share image rendering
---
web/scripts/test-hosted-local-share-ui.ts | 207 +++++++++++++++++-
web/src/BrowserReviewApp.svelte | 12 +-
web/src/hosted/app/EditorShell.svelte | 66 ++++--
web/src/lib/HtmlViewer.svelte | 26 ++-
.../lib/prosemirror/image-nodeview.test.ts | 4 +-
web/src/lib/prosemirror/image-nodeview.ts | 11 +-
web/src/lib/review/asset-resolution.test.ts | 22 +-
web/src/lib/review/asset-resolution.ts | 15 +-
.../lib/review/browser-asset-registry.test.ts | 7 +-
web/src/lib/review/browser-asset-registry.ts | 11 +
web/src/lib/review/html-shared-assets.test.ts | 9 +-
web/src/lib/review/html-shared-assets.ts | 33 ++-
web/src/lib/review/image-data-url.ts | 32 +++
.../lib/review/shared-image-policy.test.ts | 2 +
web/src/lib/review/shared-image-policy.ts | 6 +
15 files changed, 404 insertions(+), 59 deletions(-)
create mode 100644 web/src/lib/review/image-data-url.ts
diff --git a/web/scripts/test-hosted-local-share-ui.ts b/web/scripts/test-hosted-local-share-ui.ts
index 764669ec..1de38c2a 100644
--- a/web/scripts/test-hosted-local-share-ui.ts
+++ b/web/scripts/test-hosted-local-share-ui.ts
@@ -21,7 +21,7 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
import { once } from 'node:events';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
-import { chromium, type BrowserContext, type Page } from '@playwright/test';
+import { chromium, expect, type BrowserContext, type Page } from '@playwright/test';
const webRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
const relayRoot = path.resolve(webRoot, '..', 'relay');
@@ -31,12 +31,20 @@ const relayUrl = `http://127.0.0.1:${relayPort}`;
const appUrl = `http://127.0.0.1:${appPort}`;
const commentMarker = 'LOCAL-OWNER-REVIEW-COMMENT-9173';
const suggestionMarker = 'LOCAL-OWNER-REVIEW-SUGGESTION-9173';
+const sharedImageSource = '../images/pixel.png';
+const remoteImageSource = 'https://images.attn.invalid/remote-share-image.png';
+const unresolvedImageSource = 'data:;base64,';
+const sharedPng = Buffer.from(
+ 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==',
+ 'base64',
+);
const useExternalServers = process.env.ATTN_SHARE_UI_EXTERNAL === '1';
let relay: ChildProcessWithoutNullStreams | null = null;
let app: ChildProcessWithoutNullStreams | null = null;
let ownerContext: BrowserContext | null = null;
let reviewerContext: BrowserContext | null = null;
+let offlineReviewerContext: BrowserContext | null = null;
let browser: Awaited> | null = null;
const diagnostics: string[] = [];
// Playwright's locator waits use unref'd timers. Keep a real handle while the
@@ -99,6 +107,13 @@ function captureBrowserFailures(page: Page, label: string): void {
});
}
+function seedProfileDisplayName({ displayName }: { displayName: string }): void {
+ // Playwright init scripts also run inside `srcdoc` frames. Those frames are
+ // intentionally opaque-origin, where touching localStorage throws.
+ if (window.location.protocol !== 'http:' && window.location.protocol !== 'https:') return;
+ localStorage.setItem('attn.profile.displayName', displayName);
+}
+
async function selectText(page: Page, needle: string): Promise {
const result = await page.evaluate((text) => {
const view = (window as unknown as { __attnPmView?: { dom: HTMLElement } }).__attnPmView;
@@ -126,6 +141,69 @@ async function selectText(page: Page, needle: string): Promise {
await page.locator('[data-slot="selection-toolbar"]').waitFor({ state: 'visible' });
}
+async function expectResolvedSharedImage(page: Page, label: string): Promise {
+ const wrapper = page.locator(`.md-image[data-src="${sharedImageSource}"]`);
+ await wrapper.waitFor({ state: 'attached', timeout: 60_000 });
+ await page.waitForFunction(
+ (source) => document.querySelector(`.md-image[data-src="${source}"]`)?.getAttribute('data-loaded') === 'true',
+ sharedImageSource,
+ { timeout: 60_000 },
+ );
+ const image = wrapper.locator('img');
+ await image.waitFor({ state: 'visible', timeout: 60_000 });
+ const detail = await image.evaluate((element) => {
+ const imageElement = element as HTMLImageElement;
+ return {
+ src: imageElement.getAttribute('src'),
+ width: imageElement.naturalWidth,
+ height: imageElement.naturalHeight,
+ loaded: imageElement.parentElement?.getAttribute('data-loaded'),
+ };
+ });
+ if (detail.loaded !== 'true' || detail.width !== 1 || detail.height !== 1 || !detail.src?.startsWith('blob:')) {
+ throw new Error(`${label} did not render the verified local image: ${JSON.stringify(detail)}`);
+ }
+}
+
+async function expectBlockedRemoteImage(page: Page, label: string): Promise {
+ const image = page.locator(`.md-image[data-src="${remoteImageSource}"] img`);
+ await image.waitFor({ state: 'attached', timeout: 60_000 });
+ await image.waitFor({ state: 'hidden', timeout: 60_000 });
+ const detail = await image.evaluate((element) => ({
+ src: element.getAttribute('src'),
+ broken: element.parentElement?.getAttribute('data-broken'),
+ }));
+ if (detail.src !== unresolvedImageSource || detail.broken !== 'true') {
+ throw new Error(`${label} did not retain a no-network remote-image fallback: ${JSON.stringify(detail)}`);
+ }
+}
+
+async function expectResolvedSharedHtmlImages(page: Page, label: string): Promise {
+ const frame = page.frameLocator('[data-slot="html-viewer"] iframe');
+ const verified = frame.locator('#verified-html-image');
+ await expect(verified).toBeVisible({ timeout: 60_000 });
+ await expect(verified).toHaveJSProperty('naturalWidth', 1, { timeout: 60_000 });
+ await expect(verified).toHaveJSProperty('naturalHeight', 1, { timeout: 60_000 });
+ await expect(verified).toHaveAttribute('src', /^data:image\/png;base64,/u);
+
+ const pictureImage = frame.locator('#picture-html-image');
+ await expect(pictureImage).toBeVisible({ timeout: 60_000 });
+ await expect(pictureImage).toHaveJSProperty('naturalWidth', 1, { timeout: 60_000 });
+ const source = frame.locator('#verified-html-source');
+ const srcset = await source.getAttribute('srcset');
+ if (!srcset?.includes('data:image/png;base64,') || !srcset.includes(unresolvedImageSource)) {
+ throw new Error(`${label} did not rewrite picture srcset safely: ${JSON.stringify(srcset)}`);
+ }
+
+ const remote = frame.locator('#remote-html-image');
+ await expect(remote).toHaveAttribute('src', unresolvedImageSource, { timeout: 60_000 });
+ await expect(remote).toHaveJSProperty('naturalWidth', 0, { timeout: 60_000 });
+ const sandbox = await page.locator('[data-slot="html-viewer"] iframe').getAttribute('sandbox');
+ if (sandbox?.includes('allow-same-origin')) {
+ throw new Error(`${label} weakened the opaque-origin HTML sandbox: ${sandbox}`);
+ }
+}
+
async function waitForOwnerTextRebase(
page: Page,
replacement: string,
@@ -157,10 +235,11 @@ async function currentWorkspaceId(page: Page): Promise {
return id;
}
-async function createInvite(owner: Page): Promise {
+async function createInvite(owner: Page, options: { selectAll?: boolean } = {}): Promise {
await owner.locator('[data-slot="owner-header-share"]').click();
const dialog = owner.getByRole('dialog', { name: 'Share files for review' });
await dialog.waitFor({ state: 'visible' });
+ if (options.selectAll) await dialog.getByRole('button', { name: 'Select all' }).click();
await dialog.getByRole('button', { name: /Create review link/u }).click();
await dialog.locator('select[aria-label="What this link allows"]').selectOption('suggest');
const chip = dialog.locator('.share-link-chip');
@@ -216,9 +295,7 @@ async function main(): Promise {
// The product correctly asks a first-time owner to choose a display name
// after a room becomes active. This lifecycle gate is about durable review
// convergence, so provide that ordinary prerequisite before navigation.
- await ownerContext.addInitScript(() => {
- localStorage.setItem('attn.profile.displayName', 'Owner agent');
- });
+ await ownerContext.addInitScript(seedProfileDisplayName, { displayName: 'Owner agent' });
const owner = await ownerContext.newPage();
captureBrowserFailures(owner, 'owner');
await owner.goto(`${appUrl}/app#new`, { waitUntil: 'domcontentloaded' });
@@ -252,9 +329,7 @@ async function main(): Promise {
step('owner passive tab and Desk opened before review activity');
reviewerContext = await browser.newContext({ viewport: { width: 1440, height: 900 } });
- await reviewerContext.addInitScript(() => {
- localStorage.setItem('attn.profile.displayName', 'Review agent');
- });
+ await reviewerContext.addInitScript(seedProfileDisplayName, { displayName: 'Review agent' });
const reviewer = await reviewerContext.newPage();
captureBrowserFailures(reviewer, 'reviewer');
await reviewer.goto(invite, { waitUntil: 'domcontentloaded' });
@@ -363,10 +438,123 @@ async function main(): Promise {
});
step('resolved history survived reviewer disconnect and reload');
+ // Run image behavior in a fresh one-document workspace so the established
+ // comment/suggestion lifecycle above remains an independent baseline.
+ const imageOwner = await ownerContext.newPage();
+ captureBrowserFailures(imageOwner, 'image owner');
+ const remoteRequests: string[] = [];
+ imageOwner.on('request', (request) => {
+ if (request.url().startsWith(remoteImageSource)) remoteRequests.push(request.url());
+ });
+ await imageOwner.goto(`${appUrl}/app#new`, { waitUntil: 'domcontentloaded' });
+ await imageOwner.locator('input[type="file"][multiple][accept*="image"]').setInputFiles([
+ {
+ name: 'docs/review.md',
+ mimeType: 'text/markdown',
+ buffer: Buffer.from(
+ `# Hosted image share\n\n\n\n\n\n`,
+ ),
+ },
+ {
+ name: 'docs/preview.html',
+ mimeType: 'text/html',
+ buffer: Buffer.from(
+ `
+
+
+
+
+
+
+ `,
+ ),
+ },
+ { name: 'images/pixel.png', mimeType: 'image/png', buffer: sharedPng },
+ ]);
+ await imageOwner.getByRole('button', { name: 'review.md', exact: true }).waitFor({ state: 'visible' });
+ await expectResolvedSharedImage(imageOwner, 'local owner');
+ await expectBlockedRemoteImage(imageOwner, 'local owner');
+ await imageOwner.getByRole('button', { name: 'preview.html', exact: true }).click();
+ await expectResolvedSharedHtmlImages(imageOwner, 'local owner HTML document');
+ await imageOwner.getByRole('button', { name: 'review.md', exact: true }).click();
+ if (remoteRequests.length > 0) {
+ throw new Error(`local owner fetched a blocked remote image: ${JSON.stringify(remoteRequests)}`);
+ }
+ const imageInvite = await createInvite(imageOwner, { selectAll: true });
+
+ reviewerContext = await browser.newContext({ viewport: { width: 1440, height: 900 } });
+ await reviewerContext.addInitScript(seedProfileDisplayName, { displayName: 'Image review agent' });
+ const imageReviewer = await reviewerContext.newPage();
+ captureBrowserFailures(imageReviewer, 'image reviewer');
+ await imageReviewer.goto(imageInvite, { waitUntil: 'domcontentloaded' });
+ await imageReviewer.locator('[data-slot="browser-review"]').waitFor({ state: 'visible' });
+ await imageReviewer.waitForFunction(() => document.querySelector('[data-slot="browser-review"]')?.getAttribute('data-authoring-ready') === 'true');
+ await imageReviewer.getByRole('button', { name: /review\.md/u }).click();
+ await expectResolvedSharedImage(imageReviewer, 'live invited reviewer');
+ await expectBlockedRemoteImage(imageReviewer, 'live invited reviewer');
+ await imageReviewer.getByRole('button', { name: /preview\.html/u }).click();
+ await expectResolvedSharedHtmlImages(imageReviewer, 'live invited reviewer HTML document');
+
+ // A second reviewer tab owns a distinct in-memory Blob registry.
+ const follower = await reviewerContext.newPage();
+ captureBrowserFailures(follower, 'image follower');
+ await follower.goto(imageInvite, { waitUntil: 'domcontentloaded' });
+ await follower.locator('[data-slot="browser-review"]').waitFor({ state: 'visible' });
+ await follower.getByRole('button', { name: /review\.md/u }).click();
+ await expectResolvedSharedImage(follower, 'follower reviewer tab');
+ await expectBlockedRemoteImage(follower, 'follower reviewer tab');
+ await follower.close();
+
+ // Reload destroys the reviewer surface and its Blob URLs. The fresh page
+ // must hydrate and bind the asset again from the retained durable share.
+ await imageReviewer.reload({ waitUntil: 'domcontentloaded' });
+ await imageReviewer.locator('[data-slot="browser-review"]').waitFor({ state: 'visible' });
+ await imageReviewer.waitForFunction(() => document.querySelector('[data-slot="browser-review"]')?.getAttribute('data-authoring-ready') === 'true');
+ await expectResolvedSharedHtmlImages(imageReviewer, 'reloaded invited reviewer HTML document');
+ await imageReviewer.getByRole('button', { name: /review\.md/u }).click();
+ await expectResolvedSharedImage(imageReviewer, 'reloaded invited reviewer');
+ await expectBlockedRemoteImage(imageReviewer, 'reloaded invited reviewer');
+ await reviewerContext.close();
+ reviewerContext = null;
+
+ // Once every owner page is gone, a fresh reviewer has no ordinary live
+ // owner connection to borrow. The stable share must restore its document
+ // and image from the retained durable projection, then do it again after a
+ // browser reload with an empty in-memory Blob registry.
+ await ownerContext.close();
+ ownerContext = null;
+ offlineReviewerContext = await browser.newContext({ viewport: { width: 1440, height: 900 } });
+ await offlineReviewerContext.addInitScript(seedProfileDisplayName, { displayName: 'Offline review agent' });
+ const offlineReviewer = await offlineReviewerContext.newPage();
+ captureBrowserFailures(offlineReviewer, 'offline reviewer');
+ await offlineReviewer.goto(imageInvite, { waitUntil: 'domcontentloaded' });
+ const offlineShell = offlineReviewer.locator('[data-slot="browser-review"]');
+ await offlineShell.waitFor({ state: 'visible' });
+ await offlineShell.waitFor({ state: 'attached' });
+ await offlineReviewer.waitForFunction(() => document.querySelector('[data-slot="browser-review"]')?.getAttribute('data-owner-online') === 'false');
+ await offlineReviewer.getByRole('button', { name: /preview\.html/u }).click();
+ await expectResolvedSharedHtmlImages(offlineReviewer, 'owner-offline durable reviewer HTML document');
+ await offlineReviewer.getByRole('button', { name: /review\.md/u }).click();
+ await expectResolvedSharedImage(offlineReviewer, 'owner-offline durable reviewer');
+ await expectBlockedRemoteImage(offlineReviewer, 'owner-offline durable reviewer');
+ await offlineReviewer.reload({ waitUntil: 'domcontentloaded' });
+ await offlineReviewer.locator('[data-slot="browser-review"]').waitFor({ state: 'visible' });
+ await offlineReviewer.waitForFunction(() => document.querySelector('[data-slot="browser-review"]')?.getAttribute('data-owner-online') === 'false');
+ await expectResolvedSharedImage(offlineReviewer, 'reloaded owner-offline durable reviewer');
+ await expectBlockedRemoteImage(offlineReviewer, 'reloaded owner-offline durable reviewer');
+ step('hosted image share survived follower, reload, and owner-offline durable review');
+
if (diagnostics.some((line) => /\[attn drift\]/u.test(line))) {
throw new Error(`projection drift detected:\n${diagnostics.filter((line) => /\[attn drift\]/u.test(line)).join('\n')}`);
}
- const browserErrors = diagnostics.filter((line) => /(?:page error:|console error:)/u.test(line));
+ // The blocked image source intentionally uses a malformed local data URL so
+ // the existing image fallback card appears without issuing a network fetch.
+ // Chromium reports that parse failure on the console; the DOM assertions
+ // above prove it is exactly this policy fallback, not a failed shared asset.
+ const browserErrors = diagnostics.filter((line) => (
+ /(?:page error:|console error:)/u.test(line)
+ && !/console error: Failed to load resource: net::ERR_(?:INVALID_URL|FILE_NOT_FOUND)/u.test(line)
+ ));
if (browserErrors.length > 0) {
throw new Error(`browser errors detected:\n${browserErrors.join('\n')}`);
}
@@ -381,6 +569,7 @@ try {
if (relevant.length > 0) console.error(relevant.join(''));
process.exitCode = 1;
} finally {
+ await offlineReviewerContext?.close();
await reviewerContext?.close();
await ownerContext?.close();
await browser?.close();
diff --git a/web/src/BrowserReviewApp.svelte b/web/src/BrowserReviewApp.svelte
index 81513a11..de870176 100644
--- a/web/src/BrowserReviewApp.svelte
+++ b/web/src/BrowserReviewApp.svelte
@@ -61,6 +61,7 @@
import { deriveFileEntries, latestRenderableSnapshotId } from './lib/review/file-nav';
import { reviewerStatusPresentation } from './lib/review/reviewer-status-model';
import { buildSharedAssetResolver } from './lib/review/asset-resolution';
+ import { browserAssetRegistry } from './lib/review/browser-asset-registry';
import { reviewStore } from './lib/review/store.svelte';
import {
applyReviewHoverHighlight,
@@ -654,6 +655,15 @@
const docWirePath = displayedSnapshot?.ownerDisplayPath;
return buildSharedAssetResolver(snapshots, roomId, docWirePath);
});
+ const resolveReviewHtmlAssetUrl = $derived.by(() => {
+ return buildSharedAssetResolver(
+ reviewStore.snapshots,
+ sessionState.roomId,
+ displayedSnapshot?.ownerDisplayPath,
+ browserAssetRegistry,
+ 'opaque-sandbox',
+ );
+ });
$effect(() => {
void sessionState.roomId;
@@ -1706,7 +1716,7 @@
(htmlBridge = bridge)}
diff --git a/web/src/hosted/app/EditorShell.svelte b/web/src/hosted/app/EditorShell.svelte
index 88b2ff30..fdf614e4 100644
--- a/web/src/hosted/app/EditorShell.svelte
+++ b/web/src/hosted/app/EditorShell.svelte
@@ -57,6 +57,7 @@
import { peerJumpPosition } from '../../lib/peer-strip-format';
import { attachCollabPresenceSinks } from '../../lib/prosemirror/collab-presence-sinks';
import { sharedAssetPathFor } from '../../lib/review/asset-resolution';
+ import { sharedImageDataUrl } from '../../lib/review/image-data-url';
import { htmlImageSources, markdownImageSources } from '../../lib/review/document-image-sources';
import { isSupportedSharedImageMediaType } from '../../lib/review/shared-image-policy';
import LoadingLine from './LoadingLine.svelte';
@@ -381,7 +382,12 @@
// remain the source of truth and a missing/rejected asset keeps the normal
// image fallback.
let localAssetUrls = $state>({});
- let ownedAssetUrls = new Map();
+ let localHtmlAssetUrls = $state>({});
+ interface LocalAssetUrl {
+ blobUrl: string;
+ htmlDataUrl: string;
+ }
+ let ownedAssetUrls = new Map();
const resolveLocalAssetUrl = $derived.by(() => {
const documentPath = activeEntry?.path;
const urls = localAssetUrls;
@@ -391,6 +397,15 @@
return path === null ? null : urls[path] ?? null;
};
});
+ const resolveLocalHtmlAssetUrl = $derived.by(() => {
+ const documentPath = activeEntry?.path;
+ const urls = localHtmlAssetUrls;
+ if (!documentPath) return () => null;
+ return (src: string): string | null => {
+ const path = sharedAssetPathFor(documentPath, src);
+ return path === null ? null : urls[path] ?? null;
+ };
+ });
/* The empty-canvas invitation (attn-mkmz.5). A brand-new workspace lands on a
blank untitled.md, and the only standing offer to bring a real document in
@@ -834,17 +849,22 @@
});
// Resolve local image dependencies through OPFS into short-lived Blob URLs
- // for the hosted owner surface. This is intentionally separate from the
- // encrypted-share registry: local images are never staged as share
- // snapshots, and share viewers never receive an OPFS-derived URL.
+ // for Markdown and data URLs for the opaque HTML sandbox. This is
+ // intentionally separate from the encrypted-share registry: local images
+ // are never staged as share snapshots, and share viewers never receive an
+ // OPFS-derived URL.
$effect(() => {
const entry = activeEntry;
- const content = bodyText ?? displayText ?? '';
+ // `displayText` is the editor's current buffer while `bodyText` is the
+ // last route/load snapshot. Prefer the live value so a newly authored
+ // relative image resolves before the debounce commits and reloads it.
+ const content = displayText ?? bodyText ?? '';
const liveAssets = new Map(workspace.entries.map((candidate) => [candidate.path, candidate]));
if (!entry || (entry.kind !== 'markdown' && entry.kind !== 'html')) {
- for (const url of ownedAssetUrls.values()) URL.revokeObjectURL(url);
+ for (const asset of ownedAssetUrls.values()) URL.revokeObjectURL(asset.blobUrl);
ownedAssetUrls = new Map();
localAssetUrls = {};
+ localHtmlAssetUrls = {};
return;
}
const sources = entry.kind === 'markdown' ? markdownImageSources(content) : htmlImageSources(content);
@@ -858,8 +878,8 @@
}
let cancelled = false;
void (async () => {
- const next = new Map();
- const created: string[] = [];
+ const next = new Map();
+ const created: LocalAssetUrl[] = [];
try {
for (const path of paths) {
const retained = ownedAssetUrls.get(path);
@@ -872,33 +892,41 @@
const bytes = new Uint8Array(result.bytes);
try {
const mediaType = result.mediaType;
- if (!isSupportedSharedImageMediaType(mediaType)) continue;
+ if (!mediaType || !isSupportedSharedImageMediaType(mediaType)) continue;
const blob = new Blob([bytes.buffer as ArrayBuffer], { type: mediaType });
- const url = URL.createObjectURL(blob);
- created.push(url);
- next.set(path, url);
+ const asset = {
+ blobUrl: URL.createObjectURL(blob),
+ htmlDataUrl: sharedImageDataUrl(mediaType, bytes),
+ };
+ created.push(asset);
+ next.set(path, asset);
} finally {
bytes.fill(0);
}
}
if (cancelled) {
- for (const url of created) URL.revokeObjectURL(url);
+ for (const asset of created) URL.revokeObjectURL(asset.blobUrl);
return;
}
- for (const [path, url] of ownedAssetUrls) {
- if (!next.has(path)) URL.revokeObjectURL(url);
+ for (const [path, asset] of ownedAssetUrls) {
+ if (!next.has(path)) URL.revokeObjectURL(asset.blobUrl);
}
ownedAssetUrls = next;
- localAssetUrls = Object.fromEntries(next);
+ localAssetUrls = Object.fromEntries(
+ [...next].map(([path, asset]) => [path, asset.blobUrl]),
+ );
+ localHtmlAssetUrls = Object.fromEntries(
+ [...next].map(([path, asset]) => [path, asset.htmlDataUrl]),
+ );
} catch {
- for (const url of created) URL.revokeObjectURL(url);
+ for (const asset of created) URL.revokeObjectURL(asset.blobUrl);
}
})();
return () => { cancelled = true; };
});
$effect(() => () => {
- for (const url of ownedAssetUrls.values()) URL.revokeObjectURL(url);
+ for (const asset of ownedAssetUrls.values()) URL.revokeObjectURL(asset.blobUrl);
ownedAssetUrls.clear();
});
@@ -3330,7 +3358,7 @@
(htmlBridge = bridge)}
diff --git a/web/src/lib/HtmlViewer.svelte b/web/src/lib/HtmlViewer.svelte
index e518a065..5ca21974 100644
--- a/web/src/lib/HtmlViewer.svelte
+++ b/web/src/lib/HtmlViewer.svelte
@@ -172,16 +172,22 @@
-
+
+ {#key sandbox}
+
+ {/key}
{:else}
+
-
+
+ {#snippet actions()}
+ Storage
+ {/snippet}
+
+
+
+
{:else if phase === 'error'}