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'}});
}})()"#,
)
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/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.
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};
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/tests/fixtures/chooser-folder/chooser-folder.md b/tests/fixtures/chooser-folder/chooser-folder.md
new file mode 100644
index 00000000..ea939118
--- /dev/null
+++ b/tests/fixtures/chooser-folder/chooser-folder.md
@@ -0,0 +1,3 @@
+# Folder chooser fixture
+
+This file verifies that a directory picker preserves its relative path.
diff --git a/tests/fixtures/chooser-folder/deep/deep.md b/tests/fixtures/chooser-folder/deep/deep.md
new file mode 100644
index 00000000..6a5f1ee0
--- /dev/null
+++ b/tests/fixtures/chooser-folder/deep/deep.md
@@ -0,0 +1,3 @@
+# Nested folder chooser fixture
+
+This nested file verifies independent disclosure state.
diff --git a/tests/fixtures/images.md b/tests/fixtures/images.md
index 4ef9a7b6..42a9414f 100644
--- a/tests/fixtures/images.md
+++ b/tests/fixtures/images.md
@@ -1,9 +1,10 @@
# Images
-A fixture for relative image resolution in the native viewer (attn-cgev).
-Every src below is authored the way a human or an agent actually writes one;
-the viewer resolves each against **this file's own directory** and serves it
-through the `attn://` protocol handler.
+A fixture for relative image resolution in attn. Every src below is authored
+the way a human or an agent actually writes one; the viewer resolves each
+against **this file's own directory**. Native attn serves local files through
+the `attn://` protocol handler, while a browser workspace resolves the files
+that were imported with it.
## Sibling, dot-slash
@@ -25,17 +26,16 @@ blocks written inline in the markdown source.

-## Absolute URL
+## Remote source
-Anything with a scheme passes through untouched, so remote images keep working.
+An HTTPS image is a normal Markdown source. The workspace owner may load one
+directly; an invited reader chooses whether to load external images for that
+review session, so the image host cannot silently observe a reader’s request.
-The src below is deliberately unresolvable: the E2E suite asserts the exact
-string survives the resolver, and anchoring that on a live host would make the
-run fail offline and in CI. **Expect a placeholder card here** — what is being
-tested is the `src` attribute, not the pixels. To see a remote image actually
-render, swap in any live URL by hand; it will load, because nothing rewrites it.
+This image is hosted remotely. The owner workspace should request it directly;
+an invited reader should see the placeholder until they choose **Load images**.
-
+
## Missing file
diff --git a/web/e2e/hosted-shells.spec.ts b/web/e2e/hosted-shells.spec.ts
index 5c15ec32..4bd13671 100644
--- a/web/e2e/hosted-shells.spec.ts
+++ b/web/e2e/hosted-shells.spec.ts
@@ -161,7 +161,8 @@ test('a blank untitled.md opens the ordinary editor, rail and all', async ({ pag
await expect(page.locator('[data-slot="canvas-invite"]')).toHaveCount(0);
await expect(page.getByRole('textbox', { name: 'Filter files' })).toBeVisible();
await expect(page.locator('[data-path][data-active="true"]')).toContainText('untitled.md');
- await expect(page.locator('.hosted-sidebar-add')).toContainText('Add files');
+ await expect(page.locator('[data-action="add-assets"]')).toContainText('Add files');
+ await expect(page.locator('[data-action="add-folder"]')).toContainText('Add folder');
// The invitation must not arrive late — the autosave commit that follows the
// first keystroke refreshes the workspace, and that refresh used to clear the
@@ -403,8 +404,12 @@ test('asset entries render inline previews and download-only placeholders', asyn
// Safe rasters render inline from (mock-)decrypted bytes.
await expect(page.locator('.asset-image')).toBeVisible();
await page.goto('/app/w/ws-product/data/notes.json?shell=demo');
- await expect(page.locator('.hosted-native-document .eyebrow')).toHaveText('Download only');
- await expect(page.locator('.asset-preview')).toContainText('never executed');
+ await expect(page.locator('.hosted-native-document > .eyebrow')).toHaveCount(0);
+ await expect(page.locator('.download-only-card h1')).toHaveText('notes.json');
+ await expect(page.locator('.download-only-path')).toHaveText('data/notes.json');
+ await expect(page.locator('.download-only-card h2')).toHaveCount(0);
+ await expect(page.locator('.download-only-note')).toHaveText('Note: This file stays inert in the browser. Download it to use it in the right tool.');
+ await expect(page.locator('.download-only-note')).toHaveCSS('font-style', 'italic');
await expect(page.locator('.hosted-native-document').getByRole('button', { name: 'Download' })).toBeVisible();
});
diff --git a/web/hosted-share-smoke.md b/web/hosted-share-smoke.md
new file mode 100644
index 00000000..78429921
--- /dev/null
+++ b/web/hosted-share-smoke.md
@@ -0,0 +1,25 @@
+# Hosted image and import smoke test
+
+Run the app with `task dev:app`, then open `http://127.0.0.1:5173/open`.
+
+1. Choose **Files or folder** → **Folder**, select `tests/fixtures`, and open
+ `images.md` in the new workspace. The Pexels image should render for the
+ owner; `./gone.png` should remain a truthful missing-file placeholder.
+2. Open the share control, create a review link, and visit it in a private
+ second browser context. The Pexels image remains blocked until **Load
+ external images** is chosen; after opting in it renders, and a reload resets
+ the choice.
+3. In the sidebar, use the single **Add files** well. Its menu must offer
+ **Files** and **Folder**; folder imports retain nested paths. Expand and
+ collapse nested folders: closed chevrons point right, open chevrons point
+ down, and row hover/active fills stay inside the vertical folder guide.
+
+Use `/open` for a fresh import when an existing workspace already contains the
+same path. Hosted workspaces deliberately reject duplicate paths rather than
+silently overwriting durable content; the editor exposes an **Open fresh
+import** link after that conflict.
+
+The deterministic lifecycle gate is `npm run test:share-ui:live` (or
+`ATTN_SHARE_UI_EXTERNAL=1 npm run test:share-ui:live` with externally managed
+local relay and Vite servers). It covers the exact Pexels URL, reviewer consent,
+reload/offline durability, and the existing unsafe/missing-image fallbacks.
diff --git a/web/scripts/test-hosted-local-share-ui.ts b/web/scripts/test-hosted-local-share-ui.ts
index 764669ec..c0704889 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,21 @@ 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 workingRemoteImageSource = 'https://images.pexels.com/photos/35227957/pexels-photo-35227957.jpeg';
+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 +108,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 +142,186 @@ 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 expectBlockedApprovedImage(page: Page, label: string): Promise {
+ const image = page.locator(`.md-image[data-src="${workingRemoteImageSource}"] 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 gate the approved remote image before opt-in: ${JSON.stringify(detail)}`);
+ }
+}
+
+async function expectResolvedExternalImage(page: Page, label: string): Promise {
+ const wrapper = page.locator(`.md-image[data-src="${workingRemoteImageSource}"]`);
+ await wrapper.waitFor({ state: 'attached', timeout: 60_000 });
+ await page.waitForFunction(
+ (source) => document.querySelector(`.md-image[data-src="${source}"]`)?.getAttribute('data-loaded') === 'true',
+ workingRemoteImageSource,
+ { timeout: 60_000 },
+ );
+ const image = wrapper.locator('img');
+ const detail = await image.evaluate((element) => {
+ const imageElement = element as HTMLImageElement;
+ return {
+ src: imageElement.getAttribute('src'),
+ width: imageElement.naturalWidth,
+ height: imageElement.naturalHeight,
+ referrerPolicy: imageElement.getAttribute('referrerpolicy'),
+ loaded: imageElement.parentElement?.getAttribute('data-loaded'),
+ };
+ });
+ if (
+ detail.loaded !== 'true'
+ || detail.width !== 1
+ || detail.height !== 1
+ || detail.src !== workingRemoteImageSource
+ || detail.referrerPolicy !== 'no-referrer'
+ ) {
+ throw new Error(`${label} did not render the approved remote image: ${JSON.stringify(detail)}`);
+ }
+}
+
+async function chooseVisibleImport(
+ page: Page,
+ choice: 'Files' | 'Folder',
+ files: { name: string; mimeType: string; buffer: Buffer }[] | string,
+): Promise {
+ await page.getByRole('button', { name: 'Add files', exact: true }).click();
+ const menuItem = page.getByRole('menuitem', { name: new RegExp(`^${choice}\\b`, 'u') });
+ await menuItem.waitFor({ state: 'visible' });
+ const fileChooser = page.waitForEvent('filechooser');
+ await menuItem.click();
+ await (await fileChooser).setFiles(files);
+}
+
+async function routeWorkingRemoteImage(context: BrowserContext): Promise {
+ // Keep the success assertion deterministic in CI while preserving the exact
+ // authored HTTPS URL in the DOM. A manual smoke run against the app itself
+ // still exercises the real Pexels response; this route only avoids making the
+ // lifecycle gate depend on a third-party CDN.
+ await context.route(`${workingRemoteImageSource}**`, (route) => route.fulfill({
+ status: 200,
+ contentType: 'image/png',
+ body: sharedPng,
+ headers: { 'cache-control': 'no-store' },
+ }));
+}
+
+async function expectAttemptedExternalImage(page: Page, label: string): Promise {
+ const image = page.locator(`.md-image[data-src="${remoteImageSource}"] img`);
+ await image.waitFor({ state: 'attached', timeout: 60_000 });
+ await page.waitForFunction(
+ (source) => {
+ const imageElement = document.querySelector(`.md-image[data-src="${source}"] img`);
+ return imageElement?.getAttribute('src') === source
+ && imageElement.parentElement?.getAttribute('data-broken') === 'true';
+ },
+ remoteImageSource,
+ { timeout: 60_000 },
+ );
+ const detail = await image.evaluate((element) => ({
+ src: element.getAttribute('src'),
+ referrerPolicy: element.getAttribute('referrerpolicy'),
+ broken: element.parentElement?.getAttribute('data-broken'),
+ }));
+ if (
+ detail.src !== remoteImageSource
+ || detail.referrerPolicy !== 'no-referrer'
+ || detail.broken !== 'true'
+ ) {
+ throw new Error(`${label} did not attempt the approved HTTPS image safely: ${JSON.stringify(detail)}`);
+ }
+}
+
+async function expectResolvedSharedHtmlImages(
+ page: Page,
+ label: string,
+ externalImagesEnabled = false,
+): 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');
+ const expectedRemoteSrcset = externalImagesEnabled ? remoteImageSource : unresolvedImageSource;
+ if (!srcset?.includes('data:image/png;base64,') || !srcset.includes(expectedRemoteSrcset)) {
+ 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',
+ externalImagesEnabled ? remoteImageSource : unresolvedImageSource,
+ { timeout: 60_000 },
+ );
+ await expect(remote).toHaveJSProperty('naturalWidth', 0, { timeout: 60_000 });
+ await expect(remote).toHaveAttribute('referrerpolicy', 'no-referrer');
+ const approvedRemote = frame.locator('#working-remote-html-image');
+ await expect(approvedRemote).toHaveAttribute(
+ 'src',
+ externalImagesEnabled ? workingRemoteImageSource : unresolvedImageSource,
+ { timeout: 60_000 },
+ );
+ if (externalImagesEnabled) {
+ await expect(approvedRemote).toHaveJSProperty('naturalWidth', 1, { timeout: 60_000 });
+ await expect(approvedRemote).toHaveJSProperty('naturalHeight', 1, { timeout: 60_000 });
+ } else {
+ await expect(approvedRemote).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,12 +353,34 @@ 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 tierPicker = dialog.locator('select[aria-label="What this link allows"]');
+ try {
+ await tierPicker.selectOption('suggest', { timeout: 30_000 });
+ } catch (error) {
+ const resume = dialog.getByRole('button', { name: 'Resume publishing' });
+ if (await resume.isVisible().catch(() => false)) {
+ // A fresh browser workspace can race its writer lease on the first
+ // publish. Exercise the product's recoverable resume action rather
+ // than turning that transient fence into a false image-test failure.
+ await resume.click();
+ await tierPicker.selectOption('suggest', { timeout: 60_000 });
+ return finishInvite(owner, dialog);
+ }
+ throw new Error(
+ `review link did not become ready: ${JSON.stringify((await dialog.textContent() ?? '').trim())}`,
+ { cause: error },
+ );
+ }
+ return finishInvite(owner, dialog);
+}
+
+async function finishInvite(owner: Page, dialog: ReturnType): Promise {
const chip = dialog.locator('.share-link-chip');
await chip.click();
const invite = await chip.locator('code').textContent();
@@ -213,12 +431,11 @@ async function main(): Promise {
browser = await chromium.launch({ headless: true });
ownerContext = await browser.newContext({ viewport: { width: 1440, height: 900 } });
+ await routeWorkingRemoteImage(ownerContext);
// 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 +469,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 +578,190 @@ 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\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 expectResolvedExternalImage(imageOwner, 'local owner');
+ await expectAttemptedExternalImage(imageOwner, 'local owner');
+
+ // The visible rail affordance is one trigger with two explicit picker
+ // branches. Exercise both native inputs and retain a nested folder path so
+ // the tree geometry assertion below has a real branch to measure.
+ await chooseVisibleImport(imageOwner, 'Files', [{
+ name: 'chooser-file.md',
+ mimeType: 'text/markdown',
+ buffer: Buffer.from('# Files chooser\n'),
+ }]);
+ try {
+ await imageOwner.locator('[data-path$="/chooser-file.md"]').waitFor({ state: 'visible', timeout: 60_000 });
+ } catch (error) {
+ const debug = await imageOwner.evaluate(() => ({
+ paths: [...document.querySelectorAll('[data-path]')].map((element) => element.getAttribute('data-path')),
+ rail: document.querySelector('.hosted-sidebar-error')?.textContent ?? null,
+ inputs: [...document.querySelectorAll('input[type="file"]')].map((element) => ({
+ multiple: element.hasAttribute('multiple'),
+ directory: element.hasAttribute('webkitdirectory'),
+ files: (element as HTMLInputElement).files?.length ?? 0,
+ })),
+ }));
+ throw new Error(`files chooser import did not land: ${JSON.stringify(debug)}`, { cause: error });
+ }
+ await chooseVisibleImport(
+ imageOwner,
+ 'Folder',
+ path.resolve(webRoot, '..', 'tests', 'fixtures', 'chooser-folder'),
+ );
+ const nestedRow = imageOwner.locator('[data-path$="/chooser-folder/chooser-folder.md"]');
+ await nestedRow.waitFor({ state: 'visible', timeout: 60_000 });
+ const folderRow = imageOwner.locator('[data-path$="/chooser-folder"]');
+ await folderRow.waitFor({ state: 'visible', timeout: 60_000 });
+ const deepFolderRow = imageOwner.locator('[data-path$="/chooser-folder/deep"]');
+ await deepFolderRow.waitFor({ state: 'visible', timeout: 60_000 });
+ await expect(deepFolderRow).toHaveAttribute('aria-expanded', 'false');
+ await expect(deepFolderRow.locator('.sidebar-tree-chevron')).not.toHaveClass(/sidebar-tree-chevron--open/u);
+ await expect(folderRow).toHaveAttribute('aria-expanded', 'true');
+ await expect(folderRow.locator('.sidebar-tree-chevron')).toHaveClass(/sidebar-tree-chevron--open/u);
+ const nestedGeometry = await nestedRow.evaluate((element) => {
+ const style = getComputedStyle(element);
+ return { marginInlineStart: style.marginInlineStart, width: style.width };
+ });
+ if (nestedGeometry.marginInlineStart === '0px' || nestedGeometry.width === '100%') {
+ throw new Error(`nested file row was not inset from its folder guide: ${JSON.stringify(nestedGeometry)}`);
+ }
+ await folderRow.click();
+ await expect(folderRow).toHaveAttribute('aria-expanded', 'false');
+ await expect(folderRow.locator('.sidebar-tree-chevron')).not.toHaveClass(/sidebar-tree-chevron--open/u);
+ await folderRow.click();
+ await expect(folderRow).toHaveAttribute('aria-expanded', 'true');
+ await deepFolderRow.click();
+ await expect(deepFolderRow).toHaveAttribute('aria-expanded', 'true');
+ await expect(deepFolderRow.locator('.sidebar-tree-chevron')).toHaveClass(/sidebar-tree-chevron--open/u);
+ await expect(folderRow).toHaveAttribute('aria-expanded', 'true');
+ await imageOwner.getByRole('button', { name: 'preview.html', exact: true }).click();
+ await expectResolvedSharedHtmlImages(imageOwner, 'local owner HTML document', true);
+ await imageOwner.getByRole('button', { name: 'review.md', exact: true }).click();
+ if (remoteRequests.length === 0) {
+ throw new Error('local owner did not request the approved remote image');
+ }
+ const imageInvite = await createInvite(imageOwner, { selectAll: true });
+
+ reviewerContext = await browser.newContext({ viewport: { width: 1440, height: 900 } });
+ await routeWorkingRemoteImage(reviewerContext);
+ 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 expectBlockedApprovedImage(imageReviewer, 'live invited reviewer');
+ await expectBlockedRemoteImage(imageReviewer, 'live invited reviewer');
+ const loadExternalImages = imageReviewer.getByRole('button', { name: 'Load external images for this review' });
+ await loadExternalImages.waitFor({ state: 'visible' });
+ await loadExternalImages.click();
+ await expectResolvedExternalImage(imageReviewer, 'opted-in invited reviewer');
+ await expectAttemptedExternalImage(imageReviewer, 'opted-in invited reviewer');
+ await imageReviewer.getByRole('button', { name: /preview\.html/u }).click();
+ await expectResolvedSharedHtmlImages(imageReviewer, 'opted-in invited reviewer HTML document', true);
+
+ // 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 expectBlockedApprovedImage(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 expectBlockedApprovedImage(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 routeWorkingRemoteImage(offlineReviewerContext);
+ 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 expectBlockedApprovedImage(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 expectBlockedApprovedImage(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 sources use a malformed local data URL, while the approved
+ // HTTPS fixture intentionally cannot resolve. The DOM assertions above
+ // distinguish a local policy fallback from an attempted external request.
+ 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|NAME_NOT_RESOLVED)/u.test(line)
+ ));
if (browserErrors.length > 0) {
throw new Error(`browser errors detected:\n${browserErrors.join('\n')}`);
}
@@ -381,6 +776,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.external-images.test.ts b/web/src/BrowserReviewApp.external-images.test.ts
new file mode 100644
index 00000000..201f876e
--- /dev/null
+++ b/web/src/BrowserReviewApp.external-images.test.ts
@@ -0,0 +1,33 @@
+import fs from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+function assert(condition: unknown, message: string): asserts condition {
+ if (!condition) throw new Error(message);
+}
+
+const here = path.dirname(fileURLToPath(import.meta.url));
+const source = fs.readFileSync(path.join(here, 'BrowserReviewApp.svelte'), 'utf8');
+
+assert(
+ source.includes('let externalImagesEnabled = $state(false);'),
+ 'external image consent must start off for every reviewer tab',
+);
+assert(
+ (source.match(/const allowExternalImages = externalImagesEnabled;/g) ?? []).length === 2,
+ 'Markdown and HTML share resolvers must both react to reviewer consent',
+);
+assert(
+ (source.match(/if \(allowExternalImages && external !== null\) return external;/g) ?? []).length === 2,
+ 'only an explicitly enabled HTTPS image may bypass the verified-asset resolver',
+);
+assert(
+ source.includes('data-slot="browser-review-external-images"'),
+ 'a document with external images must expose a reviewer control',
+);
+assert(
+ source.includes('Those hosts may see your IP address.'),
+ 'the reviewer control must state the privacy consequence before it loads an external image',
+);
+
+console.log('browser-review external images: all assertions passed');
diff --git a/web/src/BrowserReviewApp.svelte b/web/src/BrowserReviewApp.svelte
index 21f8ec0e..cbe4de0d 100644
--- a/web/src/BrowserReviewApp.svelte
+++ b/web/src/BrowserReviewApp.svelte
@@ -60,6 +60,10 @@
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 { approvedExternalImageUrl } from './lib/review/external-image-policy';
+ import { browserAssetRegistry } from './lib/review/browser-asset-registry';
+ import { htmlImageSources, markdownImageSources } from './lib/review/document-image-sources';
import { reviewStore } from './lib/review/store.svelte';
import {
applyReviewHoverHighlight,
@@ -161,6 +165,9 @@
canRemember: true,
});
let collabSetupError = $state(null);
+ // This consent is deliberately tab/session-only. A shared document must not
+ // retain a choice that makes a later reader contact a third-party host.
+ let externalImagesEnabled = $state(false);
const authenticatedOwnerDeviceIds = new Set();
const reviewerCollabGate = new BrowserReviewerCollabGate((error) => {
collabSetupError = error.message;
@@ -643,6 +650,44 @@
? sessionState.snapshotDocType
: 'markdown';
});
+ const reviewHasExternalImages = $derived.by(() => {
+ const content = displayedContent ?? '';
+ const sources = displayedDocType === 'html'
+ ? htmlImageSources(content)
+ : markdownImageSources(content);
+ return sources.some((src) => approvedExternalImageUrl(src) !== null);
+ });
+ // 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;
+ const local = buildSharedAssetResolver(snapshots, roomId, docWirePath);
+ const allowExternalImages = externalImagesEnabled;
+ return (src: string): string | null => {
+ const external = approvedExternalImageUrl(src);
+ if (allowExternalImages && external !== null) return external;
+ return local(src);
+ };
+ });
+ const resolveReviewHtmlAssetUrl = $derived.by(() => {
+ const local = buildSharedAssetResolver(
+ reviewStore.snapshots,
+ sessionState.roomId,
+ displayedSnapshot?.ownerDisplayPath,
+ browserAssetRegistry,
+ 'opaque-sandbox',
+ );
+ const allowExternalImages = externalImagesEnabled;
+ return (src: string): string | null => {
+ const external = approvedExternalImageUrl(src);
+ if (allowExternalImages && external !== null) return external;
+ return local(src);
+ };
+ });
$effect(() => {
void sessionState.roomId;
@@ -1606,6 +1651,23 @@
{currentFileName}
+ {#if reviewHasExternalImages}
+
(externalImagesEnabled = !externalImagesEnabled)}
+ >
+ {externalImagesEnabled ? 'External images on' : 'Load images'}
+
+ {/if}
(htmlBridge = bridge)}
@@ -1703,6 +1766,7 @@
{#if phase === 'loading'}
-
+
-
+
+ {#snippet actions()}
+ Storage
+ {/snippet}
+
+
+
+
{:else if phase === 'error'}
+
@@ -3312,12 +3494,12 @@
: 'Drop a Markdown file or a folder here, or choose one.'}
- assetInput?.click()}>
- Choose files
-
- assetFolderInput?.click()}>
- Choose folder
-
+ assetInput?.click()}
+ onChooseFolder={() => assetFolderInput?.click()}
+ />
Files stay in this browser profile — nothing is uploaded.
@@ -3523,21 +3705,21 @@
thing a drag is actually aimed at — you cannot drop onto a word.
It is still one real , so the keyboard and pointer paths are
unchanged; the button is simply the size and shape of the target. -->
-
+
{/if}
{#if railError}
+ {#if importConflict}
+
+ {/if}
{/if}
{/snippet}
@@ -3936,9 +4118,12 @@
{/if}
{/each}
- assetInput?.click()}>
- + Add file or asset
-
+ assetInput?.click()}
+ onChooseFolder={() => assetFolderInput?.click()}
+ />
{/if}
diff --git a/web/src/hosted/app/ImportChooser.svelte b/web/src/hosted/app/ImportChooser.svelte
new file mode 100644
index 00000000..5c3b6221
--- /dev/null
+++ b/web/src/hosted/app/ImportChooser.svelte
@@ -0,0 +1,66 @@
+
+
+
+
+ {#if variant === 'sidebar'}
+
+
+ {#if hint}{/if}
+ {:else}
+ {label}
+ {/if}
+
+
+
diff --git a/web/src/hosted/app/OpenPage.svelte b/web/src/hosted/app/OpenPage.svelte
index f83632a4..399bb5f1 100644
--- a/web/src/hosted/app/OpenPage.svelte
+++ b/web/src/hosted/app/OpenPage.svelte
@@ -3,6 +3,7 @@
import { expandPicked, prepareImport } from './import-files';
import { fileDrop, filesToPicked, type DroppedFile } from './file-drop';
import type { ImportFileInput, StorageHealth } from './types';
+ import ImportChooser from './ImportChooser.svelte';
interface Props {
health: StorageHealth;
@@ -12,6 +13,7 @@
const { health, onImport }: Props = $props();
let fileInput = $state();
+ let folderInput = $state();
let importError = $state(null);
async function importFiles(files: Iterable): Promise {
@@ -30,6 +32,12 @@
const files = fileInput?.files;
if (files && files.length > 0) void importFiles(Array.from(files));
}
+
+ function onFolderPicked(): void {
+ const files = folderInput?.files;
+ if (files && files.length > 0) void importFiles(Array.from(files));
+ if (folderInput) folderInput.value = '';
+ }
@@ -76,9 +84,12 @@
.zip
- fileInput?.click()}>
- Choose files
-
+ fileInput?.click()}
+ onChooseFolder={() => folderInput?.click()}
+ />
+
{#if importError}
-
+
+ {#key sandbox}
+
+ {/key}
{:else}