diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index ac90933..e77eeaf 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -24,6 +24,23 @@ https://semver.org/spec/v2.0.0.html[Semantic Versioning]. distinct roles of the four contractile locations. Issues #51 and #52 are recorded as closed in the current repository checkpoint. +==== Fixed + +* *Aspect gate #49 — LSP server no longer panics on malformed requests* + (`+src/interface/lsp/src/main.rs+`): a request whose method matched but + whose params failed JSON deserialization used to crash the server with + `+panic!+`. The dispatcher now answers JSON-RPC `Invalid params` + (-32602) naming the method and the error, and the binary carries + `+#![deny(clippy::unwrap_used, clippy::expect_used)]+`, mirroring + `+vclt-gate+`. +* *Aspect gate check 3 extended to the panic family*: + `+tests/aspect_tests.sh+` now also rejects `+panic!+`, + `+unreachable!+`, `+todo!+`, and `+unimplemented!+` in production `+src/+`, + matching the documented `vcltotal-parse` SPARK-grade lint set. Remaining + `unsafe` in `+src/+` is confined to `+// SAFETY:+-justified FFI trust + boundaries and remaining `+unwrap+`/`+expect+` to test code; the gate + passes 6/6. + === [0.2.0] — 2026-06-13 ==== Added diff --git a/src/interface/lsp/src/main.rs b/src/interface/lsp/src/main.rs index 0b6d198..6cace39 100644 --- a/src/interface/lsp/src/main.rs +++ b/src/interface/lsp/src/main.rs @@ -4,12 +4,34 @@ //! This server provides LSP support for the VCL-total query language. //! Uses lsp-server (synchronous) for the transport layer. +// Binary-side mirror of the `vclt-gate` posture: the server must report +// failures to the client, never crash on them. +#![deny(clippy::unwrap_used, clippy::expect_used)] + use lsp_server::{Connection, Message, RequestId, Response}; use lsp_types::*; use std::error::Error; use vcltotal_lsp::VqlutLsp; +/// Send an LSP error response. Failures the server can attribute to a +/// request (malformed params, unserializable results) are *reported* to +/// the client — the server never crashes on them. +fn send_error( + connection: &Connection, + id: RequestId, + code: i32, + message: String, +) -> Result<(), Box> { + let resp = Response { + id, + result: None, + error: Some(lsp_server::ResponseError { code, message, data: None }), + }; + connection.sender.send(Message::Response(resp))?; + Ok(()) +} + /// Send an LSP result response, converting serialization failures to LSP /// error responses rather than panicking. This ensures the server never /// crashes on a malformed result — it reports the error to the client. @@ -24,32 +46,46 @@ fn send_result( connection.sender.send(Message::Response(resp))?; } Err(e) => { - let resp = Response { - id, - result: None, - error: Some(lsp_server::ResponseError { - code: -32603, // Internal error (JSON-RPC) - message: format!("Result serialization failed: {}", e), - data: None, - }), - }; - connection.sender.send(Message::Response(resp))?; + // Internal error (JSON-RPC) + send_error(connection, id, -32603, format!("Result serialization failed: {e}"))?; } } Ok(()) } -fn cast(req: lsp_server::Request) -> Result<(RequestId, R::Params), lsp_server::Request> +/// Outcome of attempting to read an LSP request as a typed request +/// (see `cast`); `P` is the expected params type. +enum Cast

{ + /// Method matched and params deserialized: ready to handle. + Hit(RequestId, P), + /// Method matched but params failed JSON deserialization. Carries + /// the id rescued before `extract` consumed the request, so the + /// server answers `Invalid params` instead of crashing — malformed + /// input fails closed, never panics. + BadParams { id: RequestId, method: String, error: String }, + /// Method did not match: the untouched request, for the next cast + /// attempt in the dispatch chain. + Mismatch(lsp_server::Request), +} + +fn cast(req: lsp_server::Request) -> Cast where R: lsp_types::request::Request, { - req.extract(R::METHOD).map_err(|e| match e { - lsp_server::ExtractError::MethodMismatch(req) => req, - lsp_server::ExtractError::JsonError { method: _, error: _ } => { - // Deserialization failed — treat as unhandled (cannot recover the original request) - panic!("JSON deserialization failed for LSP request") - } - }) + // `extract` consumes the request and drops the id on a params + // deserialization failure — so rescue it first. A malformed request + // is then answerable (JSON-RPC `Invalid params`, -32602) instead of + // a server crash. + let id = req.id.clone(); + match req.extract(R::METHOD) { + Ok((id, params)) => Cast::Hit(id, params), + Err(lsp_server::ExtractError::MethodMismatch(req)) => Cast::Mismatch(req), + Err(lsp_server::ExtractError::JsonError { method, error }) => Cast::BadParams { + id, + method, + error: error.to_string(), + }, + } } fn main() -> Result<(), Box> { @@ -83,22 +119,47 @@ fn main() -> Result<(), Box> { if connection.handle_shutdown(&req)? { return Ok(()); } - match cast::(req.clone()) { - Ok((id, params)) => { + match cast::(req) { + Cast::Hit(id, params) => { let result = vqlut_lsp.handle_goto_definition(params); send_result(&connection, id, &result)?; } - Err(req) => match cast::(req) { - Ok((id, params)) => { + Cast::BadParams { id, method, error } => { + // Invalid params (JSON-RPC) + send_error( + &connection, + id, + -32602, + format!("Invalid params for {method}: {error}"), + )?; + } + Cast::Mismatch(req) => match cast::(req) { + Cast::Hit(id, params) => { let result = vqlut_lsp.handle_hover(params); send_result(&connection, id, &result)?; } - Err(req) => match cast::(req) { - Ok((id, params)) => { + Cast::BadParams { id, method, error } => { + send_error( + &connection, + id, + -32602, + format!("Invalid params for {method}: {error}"), + )?; + } + Cast::Mismatch(req) => match cast::(req) { + Cast::Hit(id, params) => { let result = vqlut_lsp.handle_completion(params); send_result(&connection, id, &result)?; } - Err(req) => { + Cast::BadParams { id, method, error } => { + send_error( + &connection, + id, + -32602, + format!("Invalid params for {method}: {error}"), + )?; + } + Cast::Mismatch(req) => { eprintln!("Unhandled request: {:?}", req.method); } }, diff --git a/src/interface/parse/src/parser.rs b/src/interface/parse/src/parser.rs index ba7c75f..0bd1826 100644 --- a/src/interface/parse/src/parser.rs +++ b/src/interface/parse/src/parser.rs @@ -41,7 +41,7 @@ use crate::lexer::{Spanned, Tok}; /// sub-queries, and `parse_not → parse_not` for `NOT` chains), so an /// adversarial input of thousands of nested `(` or `NOT` would otherwise /// exhaust the native stack — a process abort (SIGABRT), which is NOT a -/// `panic!` (so the crate's `deny(clippy::panic)` cannot see it) and NOT a +/// `panic` (so the crate's `deny(clippy::panic)` cannot see it) and NOT a /// typed `ParseError`, violating the total / fail-closed contract. Past /// this bound the parser returns a typed error instead. 256 levels is far /// beyond any real query yet leaves the stack comfortably bounded. diff --git a/tests/aspect_tests.sh b/tests/aspect_tests.sh index 6c5cb90..5bcb14b 100644 --- a/tests/aspect_tests.sh +++ b/tests/aspect_tests.sh @@ -15,7 +15,9 @@ # `#[no_mangle] extern "C"` entry *cannot* be written without it), # but every `unsafe {` must carry a contiguous `// SAFETY:` # justification — mirroring `clippy::undocumented_unsafe_blocks`. -# 3. No .unwrap()/.expect() in production (non-test) Rust src/ +# 3. No .unwrap()/.expect()/panic!/unreachable!/todo!/unimplemented! +# in production (non-test) Rust src/ — the SPARK-grade fail-closed +# posture (cf. vcltotal-parse's deny lint-set, the estate pattern). # 4. HTTPS-only URLs # 5. No hardcoded secrets # 6. Totality marker: Cargo.lock committed (reproducible builds) @@ -92,14 +94,17 @@ done < <(prod_rs_files) check "No undocumented unsafe in production src/ (// SAFETY: required)" \ "$([ "$undoc_unsafe" -eq 0 ] && echo 0 || echo 1)" -# 3. No .unwrap()/.expect() in production (non-test) Rust src/. -unwrap_hits=0 +# 3. No fail-open helpers in production (non-test) Rust src/: neither +# .unwrap()/.expect() nor the panic family (`panic!`, `unreachable!`, +# `todo!`, `unimplemented!`). Mirrors the vcltotal-parse deny +# lint-set (the estate's SPARK-grade pattern). +failopen_hits=0 while IFS= read -r f; do - n=$(strip_cfg_test < "$f" | grep -c '\.unwrap()\|\.expect(' || true) - unwrap_hits=$((unwrap_hits + n)) + n=$(strip_cfg_test < "$f" | grep -c '\.unwrap()\|\.expect(\|panic!\|unreachable!\|todo!\|unimplemented!' || true) + failopen_hits=$((failopen_hits + n)) done < <(prod_rs_files) -check "No .unwrap()/.expect() in production src/" \ - "$([ "$unwrap_hits" -eq 0 ] && echo 0 || echo 1)" +check "No .unwrap()/.expect()/panic! in production src/" \ + "$([ "$failopen_hits" -eq 0 ] && echo 0 || echo 1)" # 4. HTTPS-only URLs http_hits=$(grep -rn 'http://[^l]' src/ 2>/dev/null | grep -v '#\|//' | wc -l || true)