-
Notifications
You must be signed in to change notification settings - Fork 1
Add server action registry, #[rustapi::action] macro, and /__actions dispatch with CSRF #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,96 @@ | ||||||
| //! Action registry and dispatch for server actions. | ||||||
| use crate::error::ApiError; | ||||||
| use crate::extract::{Body, Headers, Path}; | ||||||
| use crate::response::IntoResponse; | ||||||
| use crate::response::Response; | ||||||
| use bytes::Bytes; | ||||||
| use http::{header, HeaderMap}; | ||||||
| use inventory::collect; | ||||||
| use serde::de::DeserializeOwned; | ||||||
| use std::future::Future; | ||||||
| use std::pin::Pin; | ||||||
|
|
||||||
| /// The route path used for action dispatch. | ||||||
| pub const ACTIONS_PATH: &str = "/__actions/{action_id}"; | ||||||
|
|
||||||
| /// A request payload passed to action handlers. | ||||||
| #[derive(Debug, Clone)] | ||||||
| pub struct ActionRequest { | ||||||
| pub body: Bytes, | ||||||
| } | ||||||
|
|
||||||
| /// A boxed future returned by action handlers. | ||||||
| pub type ActionFuture = Pin<Box<dyn Future<Output = Response> + Send>>; | ||||||
|
|
||||||
| /// Function pointer for action handlers. | ||||||
| pub type ActionHandlerFn = fn(ActionRequest) -> ActionFuture; | ||||||
|
|
||||||
| /// Registry entry for a server action. | ||||||
| pub struct ActionDefinition { | ||||||
| pub id: &'static str, | ||||||
| pub handler: ActionHandlerFn, | ||||||
| } | ||||||
|
|
||||||
| /// Convert an incoming request body into a typed input payload. | ||||||
| pub fn decode_action_input<T: DeserializeOwned>(body: Bytes) -> Result<T, ApiError> { | ||||||
| serde_json::from_slice(&body) | ||||||
| .map_err(|err| ApiError::bad_request(format!("Invalid action payload: {}", err))) | ||||||
| } | ||||||
|
|
||||||
| collect!(ActionDefinition); | ||||||
|
|
||||||
| /// Find a registered action by id. | ||||||
| pub fn find_action(id: &str) -> Option<&'static ActionDefinition> { | ||||||
| inventory::iter::<ActionDefinition> | ||||||
| .into_iter() | ||||||
| .find(|action| action.id == id) | ||||||
| } | ||||||
|
|
||||||
| /// Handle an action POST request. | ||||||
| pub async fn action_handler( | ||||||
| Path(action_id): Path<String>, | ||||||
| Headers(headers): Headers, | ||||||
| Body(body): Body, | ||||||
| ) -> Response { | ||||||
| if let Err(err) = enforce_csrf(&headers) { | ||||||
| return err.into_response(); | ||||||
| } | ||||||
|
|
||||||
| let Some(action) = find_action(&action_id) else { | ||||||
| return ApiError::not_found(format!("Action '{}' not found", action_id)).into_response(); | ||||||
| }; | ||||||
|
|
||||||
| (action.handler)(ActionRequest { body }).await | ||||||
| } | ||||||
|
|
||||||
| fn enforce_csrf(headers: &HeaderMap) -> Result<(), ApiError> { | ||||||
| let header_token = headers | ||||||
| .get("x-csrf-token") | ||||||
| .and_then(|value| value.to_str().ok()) | ||||||
| .map(|value| value.to_string()); | ||||||
|
|
||||||
| let cookie_token = extract_cookie(headers, "csrf_token"); | ||||||
|
|
||||||
| match (cookie_token, header_token) { | ||||||
| (Some(cookie), Some(header)) if cookie == header => Ok(()), | ||||||
|
||||||
| (Some(cookie), Some(header)) if cookie == header => Ok(()), | |
| (Some(cookie), Some(header)) if cookie == header && !cookie.is_empty() => Ok(()), |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSRF token comparison uses simple string equality which may be vulnerable to timing attacks. An attacker could potentially use timing differences to determine the correct token byte-by-byte. Use a constant-time comparison function such as those provided by the subtle crate or implement a secure comparison that processes the full length regardless of when a mismatch is found.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states that CSRF is enforced, but there is no documentation explaining to users how to generate or set the CSRF token on the client side, or how to configure the csrf_token cookie. Without this information, developers won't know how to properly use this feature. Add documentation explaining the complete CSRF flow including token generation, cookie setting, and client-side header requirements.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ActionFuture type is used by the macro-generated code but is not exported in the public API. The macro at line 559 in rustapi-macros/src/lib.rs references ::rustapi_rs::ActionFuture, but this type is not re-exported through rustapi-core/src/lib.rs or rustapi-rs/src/lib.rs. This will cause compilation errors when users try to use the #[rustapi::action] macro. Add ActionFuture to the exports in both lib.rs files.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cookie parsing implementation is vulnerable to cookie injection attacks. The simple split-based parser does not properly handle quoted cookie values that may contain semicolons or equals signs. For example, a cookie value like 'session="a=b;c=d"' would be incorrectly parsed. Consider using the existing Cookies extractor from rustapi-core::extract which properly handles cookie parsing via the cookie crate, or ensure this implementation handles quoted values and edge cases correctly.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The action subsystem lacks any test coverage. Given that other modules in this crate (e.g., extract.rs, response.rs) include comprehensive test suites, this new module should also include tests for CSRF enforcement, action registration, action dispatch, cookie extraction, and error handling scenarios.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -487,6 +487,102 @@ pub fn delete(attr: TokenStream, item: TokenStream) -> TokenStream { | |
| generate_route_handler("DELETE", attr, item) | ||
| } | ||
|
|
||
| /// Register a server action handler. | ||
| /// | ||
| /// The annotated async function should accept a single input type that | ||
| /// implements `serde::Deserialize` and return a type implementing | ||
| /// `IntoResponse` (commonly `Result<Json<T>, ApiError>` or `Redirect`). | ||
| /// | ||
| /// The action will be auto-registered and can be invoked via the | ||
| /// `/__actions/{action_id}` endpoint. | ||
| #[proc_macro_attribute] | ||
| pub fn action(_attr: TokenStream, item: TokenStream) -> TokenStream { | ||
| let input = parse_macro_input!(item as ItemFn); | ||
|
|
||
| if input.sig.asyncness.is_none() { | ||
| return syn::Error::new_spanned( | ||
| &input.sig, | ||
| "#[rustapi::action] functions must be async", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
|
|
||
| if !input.sig.generics.params.is_empty() { | ||
| return syn::Error::new_spanned( | ||
| &input.sig.generics, | ||
| "#[rustapi::action] functions do not support generics", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
|
|
||
| let fn_name = &input.sig.ident; | ||
| let fn_vis = &input.vis; | ||
| let fn_attrs = &input.attrs; | ||
| let fn_inputs = &input.sig.inputs; | ||
| let fn_output = &input.sig.output; | ||
| let fn_block = &input.block; | ||
|
|
||
| let first_arg = match fn_inputs.first() { | ||
| Some(FnArg::Typed(pat_ty)) => pat_ty.ty.clone(), | ||
| _ => { | ||
| return syn::Error::new_spanned( | ||
| &input.sig.inputs, | ||
| "#[rustapi::action] requires a single typed argument", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
| }; | ||
|
|
||
| if fn_inputs.len() != 1 { | ||
| return syn::Error::new_spanned( | ||
| &input.sig.inputs, | ||
| "#[rustapi::action] requires exactly one argument", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
|
Comment on lines
+527
to
+546
|
||
|
|
||
| let action_id = fn_name.to_string(); | ||
| let registrar_name = syn::Ident::new(&format!("__RUSTAPI_ACTION_{}", fn_name), fn_name.span()); | ||
| let wrapper_name = syn::Ident::new(&format!("__rustapi_action_wrapper_{}", fn_name), fn_name.span()); | ||
|
Comment on lines
+549
to
+550
|
||
|
|
||
| let expanded = quote! { | ||
| #(#fn_attrs)* | ||
| #fn_vis async fn #fn_name(#fn_inputs) #fn_output #fn_block | ||
|
|
||
| #[doc(hidden)] | ||
| #fn_vis fn #wrapper_name( | ||
| req: ::rustapi_rs::ActionRequest, | ||
| ) -> ::rustapi_rs::ActionFuture { | ||
| Box::pin(async move { | ||
| let input: #first_arg = match ::rustapi_rs::decode_action_input(req.body) { | ||
| Ok(value) => value, | ||
| Err(err) => { | ||
| return err.into_response(); | ||
| } | ||
| }; | ||
| let result = #fn_name(input).await; | ||
| ::rustapi_rs::IntoResponse::into_response(result) | ||
| }) | ||
| } | ||
|
|
||
| #[allow(non_upper_case_globals)] | ||
| static #registrar_name: ::rustapi_rs::ActionDefinition = ::rustapi_rs::ActionDefinition { | ||
| id: #action_id, | ||
| handler: #wrapper_name, | ||
| }; | ||
|
|
||
| ::rustapi_rs::__private::inventory::submit! { #registrar_name } | ||
| }; | ||
|
|
||
| debug_output(&format!("action {}", action_id), &expanded); | ||
|
|
||
| TokenStream::from(expanded) | ||
| } | ||
|
|
||
| // ============================================ | ||
| // Route Metadata Macros | ||
| // ============================================ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The action_id parameter from the URL path is not validated or sanitized before being used. While it's used for lookup and in an error message, the lack of validation could lead to verbose error messages or logging issues. Consider adding validation to ensure action_id contains only expected characters (e.g., alphanumeric, underscores) and has reasonable length limits.