Skip to main content

max / makenotwork

mnw: bind multipart part geometry to the size declared at start (F1/F4) Deepaudit F1 (SERIOUS): multipart_start validated file_size_bytes against the tier cap but never persisted it, so multipart_parts re-derived the part plan from its own request body -- a session opened for 1 GB could mint presigned windows up to the 5 TiB MultipartPlan ceiling (unbudgeted S3 writes bounded only by the 24h reaper). - migration 174: pending_uploads gains a nullable declared_size_bytes. - start persists the tier-checked size (record_pending_multipart_upload). - parts reads it back, refuses a request whose file_size_bytes disagrees, and derives the plan from the stored size, not the request body. Also F4 (MINOR): parts now touches the pending row so the 24h orphan reaper does not abort a legitimate slow multipart transfer mid-flight. Regression test: a parts call claiming 5 TiB against a 2 GB session is refused. 16 multipart integration tests green; sqlx offline cache regenerated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-24 20:44 UTC
Commit: f83d78df91ecb3ea82e691c81e3ea14c5aea82d1
Parent: f6e5439
7 files changed, +223 insertions, -3 deletions
A server/.sqlx/query-694c618676ed1782c2ee5405be03bfc8afd6330efd22231c3991ae2c1d9d3c9e.json +24
@@ -0,0 +1,24 @@
1 + {
2 + "db_name": "PostgreSQL",
3 + "query": "SELECT declared_size_bytes FROM pending_uploads\n WHERE s3_key = $1 AND user_id = $2 AND bucket = $3",
4 + "describe": {
5 + "columns": [
6 + {
7 + "ordinal": 0,
8 + "name": "declared_size_bytes",
9 + "type_info": "Int8"
10 + }
11 + ],
12 + "parameters": {
13 + "Left": [
14 + "Text",
15 + "Uuid",
16 + "Text"
17 + ]
18 + },
19 + "nullable": [
20 + true
21 + ]
22 + },
23 + "hash": "694c618676ed1782c2ee5405be03bfc8afd6330efd22231c3991ae2c1d9d3c9e"
24 + }
A server/.sqlx/query-7e28c0c65a9f80c444ce2abbbd320dacb89feeaecccc5febcfef7b89e74327b5.json +16
@@ -0,0 +1,16 @@
1 + {
2 + "db_name": "PostgreSQL",
3 + "query": "UPDATE pending_uploads SET created_at = NOW()\n WHERE s3_key = $1 AND user_id = $2 AND bucket = $3",
4 + "describe": {
5 + "columns": [],
6 + "parameters": {
7 + "Left": [
8 + "Text",
9 + "Uuid",
10 + "Text"
11 + ]
12 + },
13 + "nullable": []
14 + },
15 + "hash": "7e28c0c65a9f80c444ce2abbbd320dacb89feeaecccc5febcfef7b89e74327b5"
16 + }
A server/.sqlx/query-a2eaee91f0df6495b0f1aaacd68acee0d485e11ca49eb1ab562d0c4e87b73ce3.json +17
@@ -0,0 +1,17 @@
1 + {
2 + "db_name": "PostgreSQL",
3 + "query": "INSERT INTO pending_uploads (user_id, s3_key, bucket, declared_size_bytes)\n VALUES ($1, $2, $3, $4)\n ON CONFLICT (s3_key, bucket) DO UPDATE\n SET created_at = NOW(), declared_size_bytes = EXCLUDED.declared_size_bytes\n WHERE pending_uploads.user_id = EXCLUDED.user_id",
4 + "describe": {
5 + "columns": [],
6 + "parameters": {
7 + "Left": [
8 + "Uuid",
9 + "Text",
10 + "Text",
11 + "Int8"
12 + ]
13 + },
14 + "nullable": []
15 + },
16 + "hash": "a2eaee91f0df6495b0f1aaacd68acee0d485e11ca49eb1ab562d0c4e87b73ce3"
17 + }
A server/migrations/174_pending_uploads_declared_size.sql +12
@@ -0,0 +1,12 @@
1 + -- Bind a multipart session's declared size to its pending-upload row.
2 + --
3 + -- Deepaudit mnw 2026-07-21 F1: `multipart_start` validated the declared
4 + -- `file_size_bytes` against the tier cap but never persisted it, so
5 + -- `multipart_parts` re-derived the part geometry from its own request body. A
6 + -- session opened for 1 GB could then request presigned windows up to the 5 TiB
7 + -- MultipartPlan ceiling -- unbudgeted S3 writes bounded only by the 24h reaper.
8 + --
9 + -- Persist the size `start` blessed so `parts` reads it from the row and refuses
10 + -- a request whose size disagrees. Nullable: single-PUT presigns and every
11 + -- pre-existing row carry NULL and are unaffected.
12 + ALTER TABLE pending_uploads ADD COLUMN declared_size_bytes BIGINT;
M server/src/db/pending_uploads.rs +71
@@ -33,6 +33,77 @@ pub(crate) async fn record_pending_upload(
33 33 Ok(())
34 34 }
35 35
36 + /// Record a pending multipart upload, persisting the size the caller declared at
37 + /// `start`. `parts` reads it back (see [`declared_size`]) to bind the part
38 + /// geometry to the tier-checked size instead of trusting a later request body.
39 + pub(crate) async fn record_pending_multipart_upload(
40 + pool: &PgPool,
41 + user_id: UserId,
42 + s3_key: &str,
43 + bucket: &str,
44 + declared_size_bytes: i64,
45 + ) -> Result<()> {
46 + // Same owner-pinned created_at refresh as record_pending_upload; also refresh
47 + // the declared size so a re-issued `start` for the same key stays consistent.
48 + sqlx::query!(
49 + "INSERT INTO pending_uploads (user_id, s3_key, bucket, declared_size_bytes)
50 + VALUES ($1, $2, $3, $4)
51 + ON CONFLICT (s3_key, bucket) DO UPDATE
52 + SET created_at = NOW(), declared_size_bytes = EXCLUDED.declared_size_bytes
53 + WHERE pending_uploads.user_id = EXCLUDED.user_id",
54 + user_id as UserId,
55 + s3_key,
56 + bucket,
57 + declared_size_bytes,
58 + )
59 + .execute(pool)
60 + .await?;
61 + Ok(())
62 + }
63 +
64 + /// The size the caller declared when it opened this pending upload, or `None`
65 + /// if the row does not exist or carries no declared size (a single-PUT presign).
66 + /// Scoped to `(user_id, s3_key, bucket)`, the same ownership proof the rest of
67 + /// this module uses.
68 + pub(crate) async fn declared_size(
69 + pool: &PgPool,
70 + user_id: UserId,
71 + s3_key: &str,
72 + bucket: &str,
73 + ) -> Result<Option<i64>> {
74 + let row = sqlx::query!(
75 + "SELECT declared_size_bytes FROM pending_uploads
76 + WHERE s3_key = $1 AND user_id = $2 AND bucket = $3",
77 + s3_key,
78 + user_id as UserId,
79 + bucket,
80 + )
81 + .fetch_optional(pool)
82 + .await?;
83 + Ok(row.and_then(|r| r.declared_size_bytes))
84 + }
85 +
86 + /// Refresh a pending upload's `created_at` so an actively-progressing transfer
87 + /// (e.g. a multipart session still requesting part URLs) is not aged out by the
88 + /// stale-pending reaper mid-flight.
89 + pub(crate) async fn touch_pending_upload(
90 + pool: &PgPool,
91 + user_id: UserId,
92 + s3_key: &str,
93 + bucket: &str,
94 + ) -> Result<()> {
95 + sqlx::query!(
96 + "UPDATE pending_uploads SET created_at = NOW()
97 + WHERE s3_key = $1 AND user_id = $2 AND bucket = $3",
98 + s3_key,
99 + user_id as UserId,
100 + bucket,
101 + )
102 + .execute(pool)
103 + .await?;
104 + Ok(())
105 + }
106 +
36 107 /// Remove the pending upload record after a successful confirm. Scoped to
37 108 /// `user_id` so a future caller that accepts a partially user-supplied key
38 109 /// can't delete another user's pending row, today's per-handler prefix
M server/src/routes/api/internal/uploads.rs +30 -3
@@ -177,7 +177,16 @@ pub(super) async fn multipart_start(
177 177 .map_err(AppError::BadRequest)?;
178 178
179 179 let s3_key = S3Client::generate_staging_key(&req.file_name);
180 - db::pending_uploads::record_pending_upload(&db, actor.user_id(), &s3_key, "main").await?;
180 + // Persist the tier-checked declared size so `multipart_parts` binds the part
181 + // geometry to it instead of trusting its own request body (deepaudit F1).
182 + db::pending_uploads::record_pending_multipart_upload(
183 + &db,
184 + actor.user_id(),
185 + &s3_key,
186 + "main",
187 + req.file_size_bytes,
188 + )
189 + .await?;
181 190
182 191 let upload_id = s3
183 192 .create_multipart_upload(&s3_key, &req.content_type)
@@ -239,8 +248,26 @@ pub(super) async fn multipart_parts(
239 248 let s3 = storage.require_s3()?;
240 249 let s3_key = authorize_multipart_key(&db, actor.user_id(), &req.s3_key).await?;
241 250
242 - let plan = s3_storage::MultipartPlan::auto(req.file_size_bytes.max(0) as u64)
243 - .map_err(AppError::BadRequest)?;
251 + // F1: the part geometry must come from the size `start` validated against the
252 + // tier cap, not from this request body. Read it back and bind the request to
253 + // it, so a session opened for 1 GB cannot mint 5 TiB of part URLs.
254 + let declared = db::pending_uploads::declared_size(&db, actor.user_id(), &req.s3_key, "main")
255 + .await?
256 + .ok_or_else(|| {
257 + AppError::BadRequest("no multipart session was started for this key".to_string())
258 + })?;
259 + if req.file_size_bytes != declared {
260 + return Err(AppError::BadRequest(format!(
261 + "file_size_bytes {} does not match the size declared at start ({declared})",
262 + req.file_size_bytes
263 + )));
264 + }
265 + // F4: this session is actively receiving parts, so refresh its liveness and
266 + // keep the 24h orphan reaper from aborting a legitimate slow transfer.
267 + db::pending_uploads::touch_pending_upload(&db, actor.user_id(), &req.s3_key, "main").await?;
268 +
269 + let plan =
270 + s3_storage::MultipartPlan::auto(declared.max(0) as u64).map_err(AppError::BadRequest)?;
244 271
245 272 if req.count == 0 || req.count > MULTIPART_PART_URL_WINDOW {
246 273 return Err(AppError::BadRequest(format!(
M server/tests/workflows/storage.rs +53
@@ -2040,6 +2040,59 @@ async fn multipart_parts_signs_each_part_with_its_exact_length() {
2040 2040 }
2041 2041
2042 2042 #[tokio::test]
2043 + async fn multipart_parts_refuses_a_size_that_disagrees_with_start() {
2044 + // deepaudit F1: the part geometry is bound to the size `start` validated
2045 + // against the tier cap. A session opened for 2 GB must not be able to widen
2046 + // itself at `parts` time -- trusting the parts body would let it mint URLs for
2047 + // a 5 TiB object, unbudgeted S3 writes bounded only by the 24h reaper.
2048 + let mut h = cli_harness().await;
2049 + let setup = h.create_creator_with_item("mpliar", "video", 0).await;
2050 + h.trust_user(setup.user_id).await;
2051 + h.grant_tier(setup.user_id, "big_files").await;
2052 + act_as(&mut h, setup.user_id);
2053 +
2054 + let declared = 2 * GIB;
2055 + let start: Value = h
2056 + .client
2057 + .post_json(
2058 + "/api/internal/upload/multipart/start",
2059 + &json!({
2060 + "item_id": setup.item_id, "file_type": "video",
2061 + "file_name": "movie.mp4", "content_type": "video/mp4",
2062 + "file_size_bytes": declared,
2063 + })
2064 + .to_string(),
2065 + )
2066 + .await
2067 + .json();
2068 + let s3_key = start["s3_key"].as_str().unwrap().to_string();
2069 + let upload_id = start["upload_id"].as_str().unwrap().to_string();
2070 +
2071 + // Claim a wildly larger size at parts time.
2072 + let resp = h
2073 + .client
2074 + .post_json(
2075 + "/api/internal/upload/multipart/parts",
2076 + &json!({
2077 + "s3_key": s3_key, "upload_id": upload_id,
2078 + "file_size_bytes": 5 * 1024 * GIB, "first_part": 1, "count": 1,
2079 + })
2080 + .to_string(),
2081 + )
2082 + .await;
2083 + assert!(
2084 + !resp.status.is_success(),
2085 + "a parts size disagreeing with start must be refused: {}",
2086 + resp.text
2087 + );
2088 + assert!(
2089 + resp.text.contains("does not match"),
2090 + "expected a declared-size mismatch error, got {}",
2091 + resp.text
2092 + );
2093 + }
2094 +
2095 + #[tokio::test]
2043 2096 async fn multipart_parts_bounds_the_window_it_will_mint() {
2044 2097 // Minting every URL for a 20 GB object would issue thousands of hour-long
2045 2098 // credentials for an upload that may never happen.