An upload asks the policy, like every other way in
/upload asked nothing. It took a file, capped it at 20 MB, and wrote it to the bucket for whoever sent it - no session, no page, no policy. It was the one server entry point that did not ask the question every other one asks, and on a site whose forms are public that makes the route public too: a free disk for anyone who finds the host. Harmless while no instance had storage configured, which is why it sat there. kasse now has a garage wired to klingenberg's portal, so it is not harmless any more. An upload now carries the question and the alternative the file is being attached to. The page has to exist, the alternative has to be open and to actually declare a `type: file` field, and the policy has to allow Submit on that page for whoever is asking - checked after the bytes are read and before any of them are written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bendik Aagaard Lynghaug
co-authored by
Claude Opus 5
Claude Opus 5
parent
9891c462c6
commit
8d6f29011c
@@ -6,6 +6,7 @@ content repos pin the tag they run (see the end of this file).
|
||||
|
||||
## Unreleased
|
||||
|
||||
- `/upload` asks the policy, like every other way in. It asked nothing: any caller who could reach the host could put 20 MB in the bucket, as often as they liked, and a site whose forms are public is a site whose upload route is public. It now takes the `question` and `alternative` the file is being attached to, refuses a page that declares no `type: file` field or an alternative that is closed, and asks `Submit` on that page - the same question the submission itself will ask - before a byte is written. Latent until an instance had storage configured; the first one now does.
|
||||
- A submitter stays in dialogue with their case for as long as it runs. `self_transition` takes `from: [state, ...]`, and the access policy compiles one rule per state named; it defaulted to the initial state alone and was written that way in the compiler, so an applicant written to for a missing drawing could not send it, and nobody could withdraw once anything had happened. Omitted, it still means the initial state alone.
|
||||
- A record can belong to another record. `type: record` with `of: <bucket>` stores the parent's id, which arrives on the link rather than being typed; a submission naming a record that is not there is refused. The other half is `where: { field, equals }` on a Kv resource, so an application's page can list the objections that name it.
|
||||
- A public view need not publish the whole bucket. `states: [...]` on a Kv resource shows only those, for one record as well as a listing - a register that is public by law is public about what was decided, not about what was withdrawn. The desk over the same bucket is a different spec and still sees all of it.
|
||||
|
||||
+11
-2
@@ -1042,7 +1042,7 @@ fn AlternativeCard(
|
||||
};
|
||||
let Some(files) = input.files() else { continue };
|
||||
let Some(file) = files.get(0) else { continue };
|
||||
match upload_file(&file).await {
|
||||
match upload_file(&file, &question_id_for_submit, &alt_name_for_submit).await {
|
||||
Ok(key) => {
|
||||
map.insert(name.clone(), serde_json::Value::String(key));
|
||||
}
|
||||
@@ -2541,12 +2541,21 @@ pub async fn submit_answer(
|
||||
/// server side is a raw Axum multipart handler (`src/upload.rs`) for
|
||||
/// the same reason.
|
||||
#[cfg(feature = "hydrate")]
|
||||
async fn upload_file(file: &web_sys::File) -> Result<String, wasm_bindgen::JsValue> {
|
||||
async fn upload_file(
|
||||
file: &web_sys::File,
|
||||
question_id: &str,
|
||||
alternative: &str,
|
||||
) -> Result<String, wasm_bindgen::JsValue> {
|
||||
use wasm_bindgen::{JsCast, JsValue};
|
||||
use wasm_bindgen_futures::JsFuture;
|
||||
use web_sys::{FormData, Request, RequestInit, Response};
|
||||
|
||||
let form = FormData::new()?;
|
||||
// Which page and which choice this file is being attached to. The
|
||||
// server refuses an upload that does not name a page it may
|
||||
// submit, so these travel with every file.
|
||||
form.append_with_str("question", question_id)?;
|
||||
form.append_with_str("alternative", alternative)?;
|
||||
form.append_with_blob("file", file)?;
|
||||
|
||||
let opts = RequestInit::new();
|
||||
|
||||
+175
-42
@@ -57,13 +57,23 @@ pub struct UploadResponse {
|
||||
key: String,
|
||||
}
|
||||
|
||||
/// POST /upload - streams the first `file` field straight through to
|
||||
/// Garage and hands back its object key. Server-side hygiene beyond the
|
||||
/// HTML `accept` hint (a UX-only signal, not a boundary): a hard size
|
||||
/// cap, and the upload is rejected outright if Garage isn't configured
|
||||
/// rather than silently succeeding nowhere.
|
||||
/// POST /upload - takes the `file` field, plus the `question` and
|
||||
/// `alternative` it belongs to, and hands back its object key.
|
||||
///
|
||||
/// The two names are not decoration: they are what makes this an
|
||||
/// authorized route rather than an open one. An upload is half a
|
||||
/// submission, so it asks the policy exactly what the submission will
|
||||
/// ask - `Submit` on that page - and it only accepts a page that
|
||||
/// actually declares a `type: file` field. Without that, anybody who
|
||||
/// could reach the host could put twenty megabytes in the bucket as
|
||||
/// often as they liked, on a site whose forms are public.
|
||||
///
|
||||
/// Server-side hygiene beyond the HTML `accept` hint (a UX signal, not
|
||||
/// a boundary): a hard size cap, and a refusal rather than a silent
|
||||
/// success when Garage isn't configured.
|
||||
pub async fn upload(
|
||||
State(state): State<AppState>,
|
||||
session: tower_sessions::Session,
|
||||
mut multipart: Multipart,
|
||||
) -> Result<Json<UploadResponse>, (StatusCode, String)> {
|
||||
let garage = state.garage.as_ref().ok_or((
|
||||
@@ -71,49 +81,172 @@ pub async fn upload(
|
||||
"uploads are not configured".to_string(),
|
||||
))?;
|
||||
|
||||
while let Some(field) = multipart
|
||||
let mut question_id = String::new();
|
||||
let mut alternative = String::new();
|
||||
let mut file: Option<(String, String, Vec<u8>)> = None;
|
||||
|
||||
while let Some(mut field) = multipart
|
||||
.next_field()
|
||||
.await
|
||||
.map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))?
|
||||
{
|
||||
if field.name() != Some("file") {
|
||||
continue;
|
||||
}
|
||||
let content_type = field
|
||||
.content_type()
|
||||
.unwrap_or("application/octet-stream")
|
||||
.to_string();
|
||||
let filename = field.file_name().unwrap_or("upload").to_string();
|
||||
// Chunk by chunk, refused the moment the cap is crossed: an
|
||||
// oversized body is never buffered whole before being turned
|
||||
// away. The route's body limit (main.rs) is the outer fence.
|
||||
let mut bytes: Vec<u8> = Vec::new();
|
||||
let mut field = field;
|
||||
while let Some(chunk) = field
|
||||
.chunk()
|
||||
.await
|
||||
.map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))?
|
||||
{
|
||||
if bytes.len() + chunk.len() > MAX_UPLOAD_BYTES {
|
||||
return Err((StatusCode::PAYLOAD_TOO_LARGE, "file too large".to_string()));
|
||||
match field.name() {
|
||||
Some("question") | Some("alternative") => {
|
||||
let name = field.name().unwrap_or_default().to_string();
|
||||
let text = field
|
||||
.text()
|
||||
.await
|
||||
.map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))?;
|
||||
if name == "question" {
|
||||
question_id = text;
|
||||
} else {
|
||||
alternative = text;
|
||||
}
|
||||
}
|
||||
bytes.extend_from_slice(&chunk);
|
||||
Some("file") => {
|
||||
let content_type = field
|
||||
.content_type()
|
||||
.unwrap_or("application/octet-stream")
|
||||
.to_string();
|
||||
let filename = field.file_name().unwrap_or("upload").to_string();
|
||||
// Chunk by chunk, refused the moment the cap is
|
||||
// crossed: an oversized body is never buffered whole
|
||||
// before being turned away. The route's body limit
|
||||
// (main.rs) is the outer fence.
|
||||
let mut bytes: Vec<u8> = Vec::new();
|
||||
while let Some(chunk) = field
|
||||
.chunk()
|
||||
.await
|
||||
.map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))?
|
||||
{
|
||||
if bytes.len() + chunk.len() > MAX_UPLOAD_BYTES {
|
||||
return Err((StatusCode::PAYLOAD_TOO_LARGE, "file too large".to_string()));
|
||||
}
|
||||
bytes.extend_from_slice(&chunk);
|
||||
}
|
||||
file = Some((filename, content_type, bytes));
|
||||
}
|
||||
_ => continue,
|
||||
}
|
||||
|
||||
let key = format!("{}-{filename}", uuid::Uuid::new_v4());
|
||||
garage
|
||||
.client
|
||||
.put_object()
|
||||
.bucket(&garage.bucket)
|
||||
.key(&key)
|
||||
.body(ByteStream::from(bytes))
|
||||
.content_type(content_type)
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| (StatusCode::BAD_GATEWAY, format!("upload failed: {e}")))?;
|
||||
|
||||
return Ok(Json(UploadResponse { key }));
|
||||
}
|
||||
|
||||
Err((StatusCode::BAD_REQUEST, "no file field in request".to_string()))
|
||||
let (filename, content_type, bytes) =
|
||||
file.ok_or((StatusCode::BAD_REQUEST, "no file field in request".to_string()))?;
|
||||
|
||||
// Nothing has been written yet. Everything below decides whether
|
||||
// anything will be.
|
||||
if !may_upload(&state, &session, &question_id, &alternative).await {
|
||||
return Err((StatusCode::FORBIDDEN, "not authorized to upload here".to_string()));
|
||||
}
|
||||
|
||||
let key = format!("{}-{filename}", uuid::Uuid::new_v4());
|
||||
garage
|
||||
.client
|
||||
.put_object()
|
||||
.bucket(&garage.bucket)
|
||||
.key(&key)
|
||||
.body(ByteStream::from(bytes))
|
||||
.content_type(content_type)
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| (StatusCode::BAD_GATEWAY, format!("upload failed: {e}")))?;
|
||||
|
||||
Ok(Json(UploadResponse { key }))
|
||||
}
|
||||
|
||||
/// Whether this caller may attach a file to that page's alternative:
|
||||
/// the alternative has to exist, has to declare a `type: file` field,
|
||||
/// and the policy has to let this principal submit that page. The same
|
||||
/// question `submit_answer` asks, asked before the bytes land rather
|
||||
/// than after.
|
||||
async fn may_upload(
|
||||
state: &AppState,
|
||||
session: &tower_sessions::Session,
|
||||
question_id: &str,
|
||||
alternative: &str,
|
||||
) -> bool {
|
||||
use crate::auth::{User, SESSION_USER_KEY};
|
||||
|
||||
let questions = state.questions.load();
|
||||
let Some(page) = page_taking_files(&questions, question_id, alternative) else {
|
||||
return false;
|
||||
};
|
||||
let user = session.get::<User>(SESSION_USER_KEY).await.ok().flatten();
|
||||
let principal = crate::access::Principal::from_user(user.as_ref());
|
||||
crate::access::decide_and_publish(
|
||||
&state.nats,
|
||||
&state.access.load(),
|
||||
&principal,
|
||||
crate::access::Action::Submit,
|
||||
&crate::access::Resource::Page { id: page },
|
||||
serde_json::json!({ "alternative": alternative }),
|
||||
)
|
||||
.allow
|
||||
}
|
||||
|
||||
/// The resolved page id, when that page has an alternative of this
|
||||
/// name that is open and actually asks for a file. `None` for a page
|
||||
/// that does not exist, an alternative that does not, one that is
|
||||
/// disabled, or one that asks for no file at all - an upload that
|
||||
/// names none of the site's own file fields is not an upload this site
|
||||
/// asked for.
|
||||
fn page_taking_files(
|
||||
questions: &std::collections::HashMap<String, crate::content::Question>,
|
||||
question_id: &str,
|
||||
alternative: &str,
|
||||
) -> Option<String> {
|
||||
let question = crate::content::resolve_question(questions, question_id)?;
|
||||
question
|
||||
.alternatives
|
||||
.iter()
|
||||
.find(|a| a.name == alternative && !a.disabled)?
|
||||
.features
|
||||
.iter()
|
||||
.any(|f| f.requirements.iter().any(|r| r.kind == "file"))
|
||||
.then(|| question.id.clone())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn questions(yaml: &str) -> std::collections::HashMap<String, crate::content::Question> {
|
||||
crate::content::build_questions(&[("index.yaml".to_string(), yaml.to_string())]).unwrap()
|
||||
}
|
||||
|
||||
const PAGE: &str = "name: Home
|
||||
alternatives:
|
||||
- name: Send drawings
|
||||
record_as: enquiries
|
||||
features:
|
||||
- name: Files
|
||||
requirements:
|
||||
- { name: drawings, type: file, optional: true }
|
||||
- name: Just ask
|
||||
features:
|
||||
- name: Words
|
||||
requirements:
|
||||
- { name: question, type: textarea }
|
||||
";
|
||||
|
||||
// The bytes are only ever written for a page of this site that
|
||||
// asked for a file. Everything else is somebody using the host as
|
||||
// a disk.
|
||||
#[test]
|
||||
fn an_upload_must_name_a_page_that_asked_for_a_file() {
|
||||
let qs = questions(PAGE);
|
||||
assert_eq!(page_taking_files(&qs, "/", "Send drawings"), Some("/".to_string()));
|
||||
// An alternative on the same page that takes no file.
|
||||
assert_eq!(page_taking_files(&qs, "/", "Just ask"), None);
|
||||
// Names that are not there at all.
|
||||
assert_eq!(page_taking_files(&qs, "/", "Nonexistent"), None);
|
||||
assert_eq!(page_taking_files(&qs, "/nowhere", "Send drawings"), None);
|
||||
assert_eq!(page_taking_files(&qs, "", ""), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_closed_alternative_takes_nothing() {
|
||||
let qs = questions(&PAGE.replace(" record_as: enquiries\n", " record_as: enquiries\n disabled: true\n"));
|
||||
assert_eq!(page_taking_files(&qs, "/", "Send drawings"), None);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user