From e0cd96ab100ca6fa4bdc9cb3170807aeec1d3b5b Mon Sep 17 00:00:00 2001 From: Zef Hemel Date: Thu, 23 Jul 2026 16:11:04 +0200 Subject: [PATCH] Fix: return proper http status and error when writing to read-only files --- docs/CHANGELOG.md | 1 + server-common/src/space/embed.rs | 20 ++++++++++---------- server-common/src/space/http.rs | 11 ++++++----- server-common/src/space/readonly.rs | 4 ++-- server-common/src/types.rs | 2 ++ server/src/handlers/mod.rs | 1 + 6 files changed, 22 insertions(+), 17 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b6d1ef91..f706ef5e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,7 @@ An attempt at documenting the changes/new features introduced in each release. Whenever a commit is pushed to the `main` branch, within ~5 minutes, it will be released as a docker image with the `:v2` tag, and a binary in the [edge release](https://github.com/silverbulletmd/silverbullet/releases/tag/edge). If you want to live on the bleeding edge of SilverBullet goodness (or regression) this is where to do it. * [[Space Manager]]: multi-space hosting with multiple accounts is here. A fresh install pointed at an empty folder opens a browser-based first-run **setup wizard** that creates an admin account and your first space, then serves it in place with no restart. One server can host any number of [[Space|spaces]], each bound to a URL prefix or hostname. +* Fix: writing to a read-only path (anything served from the bundled library, a `SB_READ_ONLY` server) returned a 500, which clients could not tell apart from a temporary server fault — so a syncing client retried it forever. Read-only refusals now return 403, and the sync engine records the path and stops re-attempting it until the local file changes. This most often bit spaces holding a stale copy of a `Library/Std` page that a later release had dropped from the bundle. * [[Baked Sections]]: bake `${...}` Lua expressions and widgets into HTML-comment-delimited markdown (`` … ``). * Space Lua: **code complete now shows documentation** (where available), all available via [[API/spacelua]] reflection APIs. diff --git a/server-common/src/space/embed.rs b/server-common/src/space/embed.rs index 56a757ec..b53dfc9f 100644 --- a/server-common/src/space/embed.rs +++ b/server-common/src/space/embed.rs @@ -149,13 +149,13 @@ impl SpacePrimitives for ReadOnlyDirSpacePrimitives { _data: &[u8], _meta: Option<&FileMeta>, ) -> Result { - Err(SpaceError::WriteError(format!( + Err(SpaceError::ReadOnly(format!( "Cannot write to read-only space: {path}" ))) } fn delete_file(&self, path: &str) -> Result<(), SpaceError> { - Err(SpaceError::WriteError(format!( + Err(SpaceError::ReadOnly(format!( "Cannot delete from read-only space: {path}" ))) } @@ -212,7 +212,7 @@ impl SpacePrimitives for FallthroughSpacePrimitives { // user-edited override — the write is allowed so it overwrites the // shadow rather than getting permanently locked. if self.primary.get_file_meta(path).is_err() && self.fallback.get_file_meta(path).is_ok() { - return Err(SpaceError::WriteError(format!( + return Err(SpaceError::ReadOnly(format!( "Cannot write file {path}: read-only" ))); } @@ -223,7 +223,7 @@ impl SpacePrimitives for FallthroughSpacePrimitives { // Same read-only enforcement as write_file: only reject when the // path exists *only* in the fallback layer. if self.primary.get_file_meta(path).is_err() && self.fallback.get_file_meta(path).is_ok() { - return Err(SpaceError::WriteError(format!( + return Err(SpaceError::ReadOnly(format!( "Cannot delete file {path}: read-only" ))); } @@ -252,12 +252,12 @@ impl SpacePrimitives for EmptySpacePrimitives { _data: &[u8], _meta: Option<&FileMeta>, ) -> Result { - Err(SpaceError::WriteError(format!( + Err(SpaceError::ReadOnly(format!( "Cannot write file {path}: read-only" ))) } fn delete_file(&self, path: &str) -> Result<(), SpaceError> { - Err(SpaceError::WriteError(format!( + Err(SpaceError::ReadOnly(format!( "Cannot delete file {path}: read-only" ))) } @@ -314,8 +314,8 @@ mod tests { .write_file("LIBRARY/foo.md", b"shadow", None) .unwrap_err(); match err { - SpaceError::WriteError(_) => {} - other => panic!("expected WriteError, got {other:?}"), + SpaceError::ReadOnly(_) => {} + other => panic!("expected ReadOnly, got {other:?}"), } assert!(!primary_td.path().join("LIBRARY/foo.md").exists()); } @@ -353,8 +353,8 @@ mod tests { let err = ft.delete_file("LIBRARY/foo.md").unwrap_err(); match err { - SpaceError::WriteError(_) => {} - other => panic!("expected WriteError, got {other:?}"), + SpaceError::ReadOnly(_) => {} + other => panic!("expected ReadOnly, got {other:?}"), } } diff --git a/server-common/src/space/http.rs b/server-common/src/space/http.rs index 843fc56c..c6fd360d 100644 --- a/server-common/src/space/http.rs +++ b/server-common/src/space/http.rs @@ -195,8 +195,9 @@ impl HttpSpacePrimitives { return Ok(()); } match status { - reqwest::StatusCode::UNAUTHORIZED | reqwest::StatusCode::FORBIDDEN => { - Err(SpaceError::Unauthorized) + reqwest::StatusCode::UNAUTHORIZED => Err(SpaceError::Unauthorized), + reqwest::StatusCode::FORBIDDEN => { + Err(SpaceError::ReadOnly(format!("{context} refused: {status}"))) } reqwest::StatusCode::NOT_FOUND => Err(SpaceError::NotFound), _ => Err(SpaceError::WriteError(format!( @@ -208,9 +209,9 @@ impl HttpSpacePrimitives { fn map_error(e: reqwest::Error) -> SpaceError { if e.status() == Some(reqwest::StatusCode::NOT_FOUND) { SpaceError::NotFound - } else if e.status() == Some(reqwest::StatusCode::UNAUTHORIZED) - || e.status() == Some(reqwest::StatusCode::FORBIDDEN) - { + } else if e.status() == Some(reqwest::StatusCode::FORBIDDEN) { + SpaceError::ReadOnly(e.to_string()) + } else if e.status() == Some(reqwest::StatusCode::UNAUTHORIZED) { SpaceError::Unauthorized } else { SpaceError::WriteError(e.to_string()) diff --git a/server-common/src/space/readonly.rs b/server-common/src/space/readonly.rs index d66dc16f..4c206749 100644 --- a/server-common/src/space/readonly.rs +++ b/server-common/src/space/readonly.rs @@ -29,12 +29,12 @@ impl SpacePrimitives for ReadOnlySpacePrimitives { _data: &[u8], _meta: Option<&FileMeta>, ) -> Result { - Err(SpaceError::WriteError(format!( + Err(SpaceError::ReadOnly(format!( "Cannot write file {path}: read-only mode" ))) } fn delete_file(&self, path: &str) -> Result<(), SpaceError> { - Err(SpaceError::WriteError(format!( + Err(SpaceError::ReadOnly(format!( "Cannot delete file {path}: read-only mode" ))) } diff --git a/server-common/src/types.rs b/server-common/src/types.rs index 4da154fd..4e147503 100644 --- a/server-common/src/types.rs +++ b/server-common/src/types.rs @@ -43,6 +43,8 @@ pub enum SpaceError { Unauthorized, #[error("Could not write file: {0}")] WriteError(String), + #[error("Read-only: {0}")] + ReadOnly(String), #[error("IO error: {0}")] Io(#[from] std::io::Error), } diff --git a/server/src/handlers/mod.rs b/server/src/handlers/mod.rs index f2e8d0c7..e8b442da 100644 --- a/server/src/handlers/mod.rs +++ b/server/src/handlers/mod.rs @@ -30,6 +30,7 @@ pub(crate) fn space_error_response(e: SpaceError) -> Response { SpaceError::NotFound => (StatusCode::NOT_FOUND, "404 page not found\n".to_string()), SpaceError::PathOutsideRoot => (StatusCode::FORBIDDEN, e.to_string()), SpaceError::Unauthorized => (StatusCode::UNAUTHORIZED, e.to_string()), + SpaceError::ReadOnly(_) => (StatusCode::FORBIDDEN, e.to_string()), _ => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()), }; Response::builder()