fix: hide SQL details in production JSON/NDJSON/SSE/CSV error responses

In production mode the HTML renderer already replaced database errors with a
generic message, but the JSON, NDJSON, SSE, and CSV renderers serialized the
full error into the response body after streaming started. That error included
the source .sql file path, the exact SQL statement, and the raw database error
text. Since the response format is selected from the Accept header, an
unauthenticated client could request an HTML page with Accept: application/json
and read back its SQL.

Thread the production flag into JsonBodyRenderer and CsvBodyRenderer and emit
the same generic message HTML uses. The full error is still logged server-side,
and detailed errors are still shown in development.

Author: lovasoa

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - **Download filenames can no longer inject extra `Content-Disposition` parameters.** The `csv` and `download` components now build the `Content-Disposition` header with a properly quoted and escaped filename instead of plain string concatenation. Before this fix, a filename containing characters such as `;`, `"`, or `=` could add a second header parameter (for example a `filename*=...` value), and some browsers prefer that injected value over the intended one. You are affected if your app puts untrusted data (such as a user-provided name or a value from the database) into the `filename` of a `csv` or `download` component. No SQL change is required: the supplied filename now always appears as a single, safely quoted `filename` value.
 - **Security fix: reserved and private files could be served directly over HTTP after a trusted page loaded them.** Files that SQLPage normally refuses to serve to direct HTTP requests (anything under the reserved `sqlpage/` prefix such as migrations, configuration, and database connection details, as well as dotfiles, parent-directory traversal paths like `../secret.sql`, and absolute paths) could briefly become reachable. This happened only after a trusted page loaded that same file with `sqlpage.run_sql(...)`, which loads files with elevated privileges and caches the parsed result. While that cache entry was fresh, a direct request such as `GET /sqlpage/secret.sql` (or the extensionless alias `GET /sqlpage/secret`) returned `200 OK` and executed the private SQL instead of returning `403 Forbidden`. Worst case, an attacker who can reach your site could read or execute internal SQL that was meant to stay private, including migration and configuration logic. You are affected if your app calls `sqlpage.run_sql()` on files inside `sqlpage/` (or on dotfiles or paths outside the web root) and is reachable by untrusted users. The fix enforces the unprivileged path guard on every direct HTTP request regardless of the cache, so these paths now always return `403 Forbidden`. Upgrade to get the fix; no configuration change is required. Legitimate `sqlpage.run_sql()` includes of such files from your own pages keep working as before.
+- **Security: production JSON, NDJSON, SSE, and CSV responses no longer leak SQL on errors.** When `environment` is set to `production`, the HTML renderer already hid database errors behind a generic message, but the JSON, NDJSON, Server-Sent Events, and CSV renderers still wrote the full error into the response body once streaming had started. That error included the path of the `.sql` file, the exact SQL statement SQLPage sent to the database, and the raw database error text. Because the response format is chosen from the request's `Accept` header, an unauthenticated visitor could request an ordinary HTML page with `Accept: application/json` and read back the SQL of pages that were never meant to expose it. You are affected if you run with `environment = production` and serve any page that can hit a database error (most apps can). There is nothing to change in your SQL: after upgrading, these formats return the same generic "Please contact the administrator for more information. The error has been logged." message that HTML already used, and the full error is still written to the server logs. In development the detailed error is still shown.
 
 ## v0.44.0
 

src/render.rs

@@ -67,6 +67,22 @@ use std::path::Path;
 use std::str::FromStr;
 use std::sync::Arc;
 
+/// Generic message shown to end users in production instead of the detailed
+/// error, which would leak the source file path, the SQL statement, and the
+/// raw database error text.
+const PRODUCTION_ERROR_MESSAGE: &str =
+    "Please contact the administrator for more information. The error has been logged.";
+
+/// Returns the message to display for an error, hiding the details in
+/// production so they are not leaked to the client.
+fn error_message(error: &anyhow::Error, is_prod: bool) -> String {
+    if is_prod {
+        PRODUCTION_ERROR_MESSAGE.to_string()
+    } else {
+        error.to_string()
+    }
+}
+
 pub enum PageContext {
     /// Indicates that we should stay in the header context
     Header(HeaderContext),
@@ -276,13 +292,14 @@ impl HeaderContext {
             };
             self.close_with_body(json_response)
         } else {
+            let is_prod = self.app_state.config.environment.is_prod();
             let body_type = get_object_str(data, "type");
             let json_renderer = match body_type {
-                None | Some("array") => JsonBodyRenderer::new_array(self.writer),
-                Some("jsonlines") => JsonBodyRenderer::new_jsonlines(self.writer),
+                None | Some("array") => JsonBodyRenderer::new_array(self.writer, is_prod),
+                Some("jsonlines") => JsonBodyRenderer::new_jsonlines(self.writer, is_prod),
                 Some("sse") => {
                     self.insert_header((header::CONTENT_TYPE, "text/event-stream"))?;
-                    JsonBodyRenderer::new_server_sent_events(self.writer)
+                    JsonBodyRenderer::new_server_sent_events(self.writer, is_prod)
                 }
                 _ => bail!(
                     "Invalid value for the 'type' property of the json component: {body_type:?}"
@@ -305,7 +322,8 @@ impl HeaderContext {
             let extension = if filename.contains('.') { "" } else { ".csv" };
             self.insert_header(attachment_with_filename(&format!("{filename}{extension}")))?;
         }
-        let csv_renderer = CsvBodyRenderer::new(self.writer, options).await?;
+        let is_prod = self.app_state.config.environment.is_prod();
+        let csv_renderer = CsvBodyRenderer::new(self.writer, is_prod, options).await?;
         let renderer = AnyRenderBodyContext::Csv(csv_renderer);
         let http_response = self.response.take();
         Ok(PageContext::Body {
@@ -396,12 +414,13 @@ impl HeaderContext {
 
     async fn start_body(mut self, data: JsonValue) -> anyhow::Result<PageContext> {
         self.add_server_timing_header()?;
+        let is_prod = self.app_state.config.environment.is_prod();
         let renderer = match self.request_context.response_format {
             ResponseFormat::Json => AnyRenderBodyContext::Json(
-                JsonBodyRenderer::new_array_with_first_row(self.writer, &data),
+                JsonBodyRenderer::new_array_with_first_row(self.writer, is_prod, &data),
             ),
             ResponseFormat::JsonLines => AnyRenderBodyContext::Json(
-                JsonBodyRenderer::new_jsonlines_with_first_row(self.writer, &data),
+                JsonBodyRenderer::new_jsonlines_with_first_row(self.writer, is_prod, &data),
             ),
             ResponseFormat::Html => {
                 let html_renderer =
@@ -528,48 +547,60 @@ impl AnyRenderBodyContext {
 pub struct JsonBodyRenderer<W: std::io::Write> {
     writer: W,
     is_first: bool,
+    is_prod: bool,
     prefix: &'static [u8],
     suffix: &'static [u8],
     separator: &'static [u8],
 }
 
 impl<W: std::io::Write> JsonBodyRenderer<W> {
-    pub fn new_array(writer: W) -> JsonBodyRenderer<W> {
+    pub fn new_array(writer: W, is_prod: bool) -> JsonBodyRenderer<W> {
         let mut renderer = Self {
             writer,
             is_first: true,
+            is_prod,
             prefix: b"[\n",
             suffix: b"\n]",
             separator: b",\n",
         };
         let _ = renderer.write_prefix();
         renderer
     }
-    pub fn new_array_with_first_row(writer: W, first_row: &JsonValue) -> JsonBodyRenderer<W> {
-        let mut renderer = Self::new_array(writer);
+    pub fn new_array_with_first_row(
+        writer: W,
+        is_prod: bool,
+        first_row: &JsonValue,
+    ) -> JsonBodyRenderer<W> {
+        let mut renderer = Self::new_array(writer, is_prod);
         let _ = renderer.handle_row(first_row);
         renderer
     }
-    pub fn new_jsonlines(writer: W) -> JsonBodyRenderer<W> {
+    pub fn new_jsonlines(writer: W, is_prod: bool) -> JsonBodyRenderer<W> {
         let mut renderer = Self {
             writer,
             is_first: true,
+            is_prod,
             prefix: b"",
             suffix: b"",
             separator: b"\n",
         };
         renderer.write_prefix().unwrap();
         renderer
     }
-    pub fn new_jsonlines_with_first_row(writer: W, first_row: &JsonValue) -> JsonBodyRenderer<W> {
-        let mut renderer = Self::new_jsonlines(writer);
+    pub fn new_jsonlines_with_first_row(
+        writer: W,
+        is_prod: bool,
+        first_row: &JsonValue,
+    ) -> JsonBodyRenderer<W> {
+        let mut renderer = Self::new_jsonlines(writer, is_prod);
         let _ = renderer.handle_row(first_row);
         renderer
     }
-    pub fn new_server_sent_events(writer: W) -> JsonBodyRenderer<W> {
+    pub fn new_server_sent_events(writer: W, is_prod: bool) -> JsonBodyRenderer<W> {
         let mut renderer = Self {
             writer,
             is_first: true,
+            is_prod,
             prefix: b"data: ",
             suffix: b"\n\n",
             separator: b"\n\ndata: ",
@@ -592,7 +623,7 @@ impl<W: std::io::Write> JsonBodyRenderer<W> {
     }
     pub fn handle_error(&mut self, error: &anyhow::Error) -> anyhow::Result<()> {
         self.handle_row(&json!({
-            "error": error.to_string()
+            "error": error_message(error, self.is_prod)
         }))
     }
 
@@ -606,11 +637,13 @@ pub struct CsvBodyRenderer {
     // The writer is a large struct, so we store it on the heap
     writer: Box<csv_async::AsyncWriter<AsyncResponseWriter>>,
     columns: Vec<String>,
+    is_prod: bool,
 }
 
 impl CsvBodyRenderer {
     pub async fn new(
         mut writer: ResponseWriter,
+        is_prod: bool,
         options: &JsonValue,
     ) -> anyhow::Result<CsvBodyRenderer> {
         let mut builder = csv_async::AsyncWriterBuilder::new();
@@ -646,6 +679,7 @@ impl CsvBodyRenderer {
         Ok(CsvBodyRenderer {
             writer: Box::new(writer),
             columns: vec![],
+            is_prod,
         })
     }
 
@@ -678,13 +712,13 @@ impl CsvBodyRenderer {
     }
 
     pub async fn handle_error(&mut self, error: &anyhow::Error) -> anyhow::Result<()> {
-        let err_str = error.to_string();
+        let err_str = error_message(error, self.is_prod);
         self.writer
             .write_record(
                 self.columns
                     .iter()
                     .enumerate()
-                    .map(|(i, _)| if i == 0 { &err_str } else { "" })
+                    .map(|(i, _)| if i == 0 { err_str.as_str() } else { "" })
                     .collect::<Vec<_>>(),
             )
             .await?;
@@ -854,7 +888,7 @@ impl<W: std::io::Write> HtmlRenderContext<W> {
         self.close_component()?;
         let data = if self.app_state.config.environment.is_prod() {
             json!({
-                "description": format!("Please contact the administrator for more information. The error has been logged."),
+                "description": PRODUCTION_ERROR_MESSAGE,
             })
         } else {
             json!({

tests/data_formats/csv_error_leak.sql

@@ -0,0 +1,3 @@
+select 'csv' as component;
+select 0 as id, 'before the error' as msg;
+select * from definitely_missing_table_xyz;

tests/data_formats/json_error_leak.sql

@@ -0,0 +1,3 @@
+select 'json' as component;
+select 'before the error' as message;
+select * from definitely_missing_table_xyz;

tests/data_formats/mod.rs

@@ -231,3 +231,96 @@ async fn test_accept_json_redirect_still_works() -> actix_web::Result<()> {
     );
     Ok(())
 }
+
+/// Builds an `AppState` running in production mode.
+async fn make_prod_app_data() -> actix_web::web::Data<sqlpage::AppState> {
+    crate::common::init_log();
+    let mut config = crate::common::test_config();
+    config.environment = sqlpage::app_config::DevOrProd::Production;
+    crate::common::make_app_data_from_config(config).await
+}
+
+async fn req_prod_with_accept(path: &str, accept: &str) -> String {
+    let app_data = make_prod_app_data().await;
+    let req = TestRequest::get()
+        .uri(path)
+        .insert_header((header::ACCEPT, accept))
+        .app_data(app_data)
+        .to_srv_request();
+    let resp = main_handler(req)
+        .await
+        .expect("request should not fail at the handler level");
+    let body = test::read_body(resp).await;
+    String::from_utf8(body.to_vec()).unwrap()
+}
+
+/// In production, a SQL error that happens mid-stream must not leak the SQL
+/// statement, the source file path, or the raw database error text.
+fn assert_no_sql_leak(body: &str, context: &str) {
+    for needle in [
+        "definitely_missing_table_xyz",
+        "The SQL statement sent by SQLPage",
+        "error_leak.sql",
+        ".sql\"",
+    ] {
+        assert!(
+            !body.contains(needle),
+            "production error response leaked {needle:?} in {context}: {body}"
+        );
+    }
+    assert!(
+        body.to_lowercase().contains("administrator"),
+        "production error response should contain a generic message in {context}: {body}"
+    );
+}
+
+#[actix_web::test]
+async fn test_prod_json_error_does_not_leak_sql() {
+    let body = req_prod_with_accept(
+        "/tests/data_formats/json_error_leak.sql",
+        "application/json",
+    )
+    .await;
+    assert!(
+        body.contains("before the error"),
+        "the good row should still be streamed: {body}"
+    );
+    assert_no_sql_leak(&body, "json error");
+}
+
+#[actix_web::test]
+async fn test_prod_csv_error_does_not_leak_sql() {
+    let app_data = make_prod_app_data().await;
+    if matches!(
+        app_data.db.info.database_type,
+        sqlpage::webserver::database::SupportedDatabase::Oracle
+    ) {
+        return;
+    }
+    let req = TestRequest::get()
+        .uri("/tests/data_formats/csv_error_leak.sql")
+        .insert_header((header::ACCEPT, "text/csv"))
+        .app_data(app_data)
+        .to_srv_request();
+    let resp = main_handler(req).await.expect("handler should not fail");
+    let body = test::read_body(resp).await;
+    let body = String::from_utf8(body.to_vec()).unwrap();
+    assert!(
+        body.contains("before the error"),
+        "the good row should still be streamed: {body}"
+    );
+    assert_no_sql_leak(&body, "csv error");
+}
+
+/// An author may only intend a page to be served as HTML, but a client can
+/// request it with `Accept: application/json` and pick the JSON renderer.
+/// In production that path must not leak SQL text either.
+#[actix_web::test]
+async fn test_prod_html_page_requested_as_json_does_not_leak_sql() {
+    let body = req_prod_with_accept(
+        "/tests/data_formats/text_error_leak.sql",
+        "application/json",
+    )
+    .await;
+    assert_no_sql_leak(&body, "text page requested as json");
+}

tests/data_formats/text_error_leak.sql

@@ -0,0 +1,2 @@
+select 'text' as component, 'before the error' as contents;
+select * from definitely_missing_table_xyz;