Skip to main content

max / makenotwork

Add an in-process S3 stub and cover upload authz with it The upload handler returns 503 when AppState.s3 is None, and every harness passed None, so the whole handler past that guard was untested: authz, rate limit, image validation, EXIF strip, and the insert-row-before-upload ordering. The stub is an axum server on an ephemeral loopback port backed by a HashMap, serving path-style PUT/GET/DELETE. That is enough because the shared s3_storage client sets force_path_style(true) and runs no connectivity probe at construction. It does not verify SigV4: the SDK signs the request, and re-checking the signature would test the AWS SDK rather than the forum. Opt-in via HarnessOptions::s3 so upload_returns_503_without_s3 keeps covering the unconfigured path. Three tests: a baseline that asserts the object round-trips against the s3_key on the images row, so the two 403 cases cannot pass on a broken stub, plus suspended and muted uploads. Both 403 cases return 200 with the authz fix reverted, storing the object and returning a hosted URL.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-28 01:29 UTC
Signed with PGP, not checked
Commit: 169c778ae313a910755a47b47293ac09681577c4
Parent: 1cd2823
3 files changed, +310 insertions, -2 deletions
@@ -2,6 +2,7 @@
2 2
3 3 pub(crate) mod client;
4 4 pub(crate) mod db;
5 + pub(crate) mod s3;
5 6
6 7 use multithreaded::{AppState, config::Config, csrf, routes};
7 8 use sqlx::PgPool;
@@ -12,10 +13,14 @@
12 13
13 14 use self::client::TestClient;
14 15 use self::db::TestDb;
16 + use self::s3::S3Stub;
15 17
16 18 pub(crate) struct TestHarness {
17 19 pub client: TestClient,
18 20 pub db: PgPool,
21 + /// The in-process object store, present only when `HarnessOptions::s3` was
22 + /// set. Tests assert against it to prove an upload did (or did not) land.
23 + pub s3: Option<S3Stub>,
19 24 _test_db: TestDb,
20 25 }
21 26
@@ -30,6 +35,9 @@
30 35 /// tests can drive `/img-proxy` through the production fetch path. Off by
31 36 /// default (post-creation preview fetches stay inert in the common case).
32 37 pub link_preview_http: bool,
38 + /// Start an in-process S3 stub and point the app at it. Off by default, so
39 + /// `upload_returns_503_without_s3` still exercises the unconfigured path.
40 + pub s3: bool,
33 41 }
34 42
35 43 impl TestHarness {
@@ -54,6 +62,21 @@
54 62 tower_sessions::cookie::time::Duration::days(1),
55 63 ));
56 64
65 + let s3_stub = if opts.s3 {
66 + Some(S3Stub::start().await)
67 + } else {
68 + None
69 + };
70 + let s3_config = s3_stub
71 + .as_ref()
72 + .map(|stub| multithreaded::config::S3Config {
73 + endpoint: stub.endpoint.clone(),
74 + bucket: stub.bucket.clone(),
75 + access_key: "test-access-key".to_string(),
76 + secret_key: "test-secret-key".to_string(),
77 + region: "us-east-1".to_string(),
78 + });
79 +
57 80 let config = Config {
58 81 mnw_base_url: opts
59 82 .mnw_base_url
@@ -64,7 +87,7 @@
64 87 oauth_redirect_uri: "http://127.0.0.1:3400/auth/callback".to_string(),
65 88 platform_admin_id: opts.platform_admin_id,
66 89 cookie_secure: false,
67 - s3: None,
90 + s3: s3_config.clone(),
68 91 internal_shared_secret: None,
69 92 trusted_proxies: std::sync::Arc::from([std::net::IpAddr::from([127, 0, 0, 1])]),
70 93 };
@@ -82,7 +105,14 @@
82 105 } else {
83 106 multithreaded::link_preview::LinkPreviewFetcher::Noop
84 107 },
85 - s3: None,
108 + s3: match &s3_config {
109 + Some(cfg) => Some(std::sync::Arc::new(
110 + multithreaded::storage::S3Storage::new(cfg)
111 + .await
112 + .expect("failed to build S3 client against the stub"),
113 + )),
114 + None => None,
115 + },
86 116 };
87 117
88 118 // Build the app with a /_test/login route for setting sessions without OAuth
@@ -100,10 +130,21 @@
100 130 TestHarness {
101 131 client,
102 132 db: pool,
133 + s3: s3_stub,
103 134 _test_db: test_db,
104 135 }
105 136 }
106 137
138 + /// Create a harness backed by the in-process S3 stub, so upload handlers
139 + /// run past the `state.s3` guard.
140 + pub(crate) async fn new_with_s3() -> Self {
141 + Self::with_options(HarnessOptions {
142 + s3: true,
143 + ..Default::default()
144 + })
145 + .await
146 + }
147 +
107 148 /// Log in as a user by username. Creates the user if needed. Returns the user's UUID.
108 149 pub(crate) async fn login_as(&mut self, username: &str) -> Uuid {
109 150 let user_id = Uuid::new_v4();
@@ -1,4 +1,13 @@
1 1 use crate::harness::TestHarness;
2 + use mt_core::types::BanType;
3 +
4 + /// Magic bytes plus padding: enough for `validate_image`'s sniff, and the zeroed
5 + /// IHDR reads back as 0x0 so the pixel-bomb cap is not involved.
6 + fn png_bytes() -> Vec<u8> {
7 + let mut v = vec![0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A];
8 + v.extend_from_slice(&[0u8; 64]);
9 + v
10 + }
2 11
3 12 #[tokio::test]
4 13 async fn upload_requires_login() {
@@ -77,3 +86,116 @@
77 86 "anonymous image-proxy request must be rejected before any fetch"
78 87 );
79 88 }
89 +
90 + // Upload authz, driven against the in-process S3 stub. Without it these
91 + // handlers stop at the `state.s3` 503 guard and never reach the gate.
92 +
93 + /// Baseline: with S3 wired, a member in good standing gets through and the
94 + /// object actually lands. The 403 tests below are only meaningful next to this
95 + /// one, since a broken stub would make every case fail for the wrong reason.
96 + #[tokio::test]
97 + async fn upload_stores_object_for_member_in_good_standing() {
98 + let mut h = TestHarness::new_with_s3().await;
99 + let user_id = h.login_as("okuploader").await;
100 + let comm_id = h.create_community("UploadComm", "upload-comm").await;
101 + h.add_membership(user_id, comm_id, "member").await;
102 +
103 + let resp = h
104 + .client
105 + .post_multipart(
106 + "/p/upload-comm/upload",
107 + &png_bytes(),
108 + "image/png",
109 + "test.png",
110 + )
111 + .await;
112 + assert_eq!(resp.status.as_u16(), 200, "member upload should succeed");
113 +
114 + let stub = h.s3.as_ref().expect("harness built with S3");
115 + assert_eq!(stub.len(), 1, "exactly one object should have been stored");
116 +
117 + let row = sqlx::query_scalar::<_, String>("SELECT s3_key FROM images LIMIT 1")
118 + .fetch_one(&h.db)
119 + .await
120 + .expect("image row should exist");
121 + let stored = stub
122 + .get(&row)
123 + .expect("stored object should match the row key");
124 + assert_eq!(stored.data, png_bytes(), "stored bytes should round-trip");
125 + assert_eq!(stored.content_type, "image/png");
126 + }
127 +
128 + #[tokio::test]
129 + async fn platform_suspended_user_cannot_upload() {
130 + let mut h = TestHarness::new_with_s3().await;
131 + let user_id = h.login_as("suspuploader").await;
132 + let comm_id = h.create_community("UploadComm", "upload-comm").await;
133 + h.add_membership(user_id, comm_id, "member").await;
134 +
135 + mt_db::mutations::suspend_user(&h.db, user_id, Some("test"))
136 + .await
137 + .unwrap();
138 +
139 + let resp = h
140 + .client
141 + .post_multipart(
142 + "/p/upload-comm/upload",
143 + &png_bytes(),
144 + "image/png",
145 + "test.png",
146 + )
147 + .await;
148 + assert_eq!(
149 + resp.status.as_u16(),
150 + 403,
151 + "a platform-suspended user must not be able to upload"
152 + );
153 + assert_eq!(
154 + h.s3.as_ref().expect("harness built with S3").len(),
155 + 0,
156 + "no object should have been stored"
157 + );
158 + }
159 +
160 + #[tokio::test]
161 + async fn muted_user_cannot_upload() {
162 + let mut h = TestHarness::new_with_s3().await;
163 + let owner_id = h.login_as("uploadowner").await;
164 + let comm_id = h.create_community("UploadComm", "upload-comm").await;
165 + h.add_membership(owner_id, comm_id, "owner").await;
166 +
167 + let user_id = h.login_as("muteuploader").await;
168 + h.add_membership(user_id, comm_id, "member").await;
169 +
170 + mt_db::mutations::create_community_ban(
171 + &h.db,
172 + comm_id,
173 + user_id,
174 + owner_id,
175 + BanType::Mute,
176 + Some("test"),
177 + None,
178 + )
179 + .await
180 + .unwrap();
181 +
182 + let resp = h
183 + .client
184 + .post_multipart(
185 + "/p/upload-comm/upload",
186 + &png_bytes(),
187 + "image/png",
188 + "test.png",
189 + )
190 + .await;
191 + assert_eq!(
192 + resp.status.as_u16(),
193 + 403,
194 + "a muted user must not be able to upload"
195 + );
196 + assert_eq!(
197 + h.s3.as_ref().expect("harness built with S3").len(),
198 + 0,
199 + "no object should have been stored"
200 + );
201 + }
@@ -1,0 +1,145 @@
1 + //! Minimal in-process S3-compatible object store for upload tests.
2 + //!
3 + //! The upload handler bails with 503 when `AppState.s3` is `None`, which put
4 + //! everything past that line (authz, rate limit, validation, EXIF strip, the
5 + //! insert-before-upload ordering) out of reach of the integration suite. This
6 + //! stub closes that gap without a container or a live bucket.
7 + //!
8 + //! It implements exactly what `s3_storage::S3Client` sends on the paths the
9 + //! forum uses: path-style PUT, GET, and DELETE of a single object. The SDK is
10 + //! built with `force_path_style(true)` and performs no connectivity probe at
11 + //! construction, so pointing `endpoint` at this server is enough. Requests are
12 + //! not signature-checked: the client signs them, and re-verifying SigV4 here
13 + //! would test the AWS SDK rather than the forum.
14 +
15 + use std::collections::HashMap;
16 + use std::sync::{Arc, Mutex};
17 +
18 + use axum::{
19 + Router,
20 + body::Bytes,
21 + extract::{Path, State},
22 + http::{HeaderMap, StatusCode, header},
23 + response::{IntoResponse, Response},
24 + routing::any,
25 + };
26 +
27 + /// An object as the stub holds it.
28 + #[derive(Clone)]
29 + pub(crate) struct StoredObject {
30 + pub data: Vec<u8>,
31 + pub content_type: String,
32 + }
33 +
34 + type Objects = Arc<Mutex<HashMap<String, StoredObject>>>;
35 +
36 + /// A running stub. Dropping the harness drops the server task with it.
37 + pub(crate) struct S3Stub {
38 + pub endpoint: String,
39 + pub bucket: String,
40 + objects: Objects,
41 + }
42 +
43 + impl S3Stub {
44 + /// Bind on an ephemeral loopback port and serve until dropped.
45 + pub(crate) async fn start() -> Self {
46 + let objects: Objects = Arc::new(Mutex::new(HashMap::new()));
47 + let bucket = "test-bucket".to_string();
48 +
49 + let app = Router::new()
50 + .route("/{bucket}/{*key}", any(object_handler))
51 + .with_state(objects.clone());
52 +
53 + let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
54 + .await
55 + .expect("failed to bind S3 stub");
56 + let addr = listener.local_addr().expect("no local addr for S3 stub");
57 +
58 + tokio::spawn(async move {
59 + let _ = axum::serve(listener, app).await;
60 + });
61 +
62 + S3Stub {
63 + endpoint: format!("http://{addr}"),
64 + bucket,
65 + objects,
66 + }
67 + }
68 +
69 + /// Number of objects currently stored.
70 + pub(crate) fn len(&self) -> usize {
71 + self.objects.lock().expect("S3 stub mutex poisoned").len()
72 + }
73 +
74 + /// Fetch a stored object by key.
75 + pub(crate) fn get(&self, key: &str) -> Option<StoredObject> {
76 + self.objects
77 + .lock()
78 + .expect("S3 stub mutex poisoned")
79 + .get(key)
80 + .cloned()
81 + }
82 + }
83 +
84 + /// One handler for the three verbs, so an unexpected method surfaces as 405
85 + /// rather than a routing miss that the SDK would report as a connection error.
86 + async fn object_handler(
87 + State(objects): State<Objects>,
88 + Path((_bucket, key)): Path<(String, String)>,
89 + method: axum::http::Method,
90 + headers: HeaderMap,
91 + body: Bytes,
92 + ) -> Response {
93 + match method {
94 + axum::http::Method::PUT => {
95 + let content_type = headers
96 + .get(header::CONTENT_TYPE)
97 + .and_then(|v| v.to_str().ok())
98 + .unwrap_or("application/octet-stream")
99 + .to_string();
100 + objects.lock().expect("S3 stub mutex poisoned").insert(
101 + key,
102 + StoredObject {
103 + data: body.to_vec(),
104 + content_type,
105 + },
106 + );
107 + // A real PutObject answers 200 with an ETag and no body.
108 + ([(header::ETAG, "\"stub\"")], StatusCode::OK).into_response()
109 + }
110 + axum::http::Method::GET => {
111 + let found = objects
112 + .lock()
113 + .expect("S3 stub mutex poisoned")
114 + .get(&key)
115 + .cloned();
116 + match found {
117 + Some(obj) => (
118 + [
119 + (header::CONTENT_TYPE, obj.content_type),
120 + (header::ETAG, "\"stub\"".to_string()),
121 + ],
122 + obj.data,
123 + )
124 + .into_response(),
125 + None => not_found(),
126 + }
127 + }
128 + axum::http::Method::DELETE => {
129 + objects.lock().expect("S3 stub mutex poisoned").remove(&key);
130 + StatusCode::NO_CONTENT.into_response()
131 + }
132 + _ => StatusCode::METHOD_NOT_ALLOWED.into_response(),
133 + }
134 + }
135 +
136 + /// S3 reports a missing key as 404 carrying a `NoSuchKey` error document; the
137 + /// SDK parses the body to classify the error, so return the real shape.
138 + fn not_found() -> Response {
139 + (
140 + StatusCode::NOT_FOUND,
141 + [(header::CONTENT_TYPE, "application/xml")],
142 + r#"<?xml version="1.0" encoding="UTF-8"?><Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message></Error>"#,
143 + )
144 + .into_response()
145 + }