fix(security): document unsafe and TLS overrides (#2835)

.config/make/lint-fmt.mak

@@ -16,6 +16,11 @@ clippy-check: core-deps ## Run clippy checks
 	cargo clippy --fix --allow-dirty
 	cargo clippy --all-targets --all-features -- -D warnings
 
+.PHONY: unsafe-code-check
+unsafe-code-check: ## Check unsafe_code allowances have SAFETY comments
+	@echo "🔒 Checking unsafe_code allowances..."
+	./scripts/check_unsafe_code_allowances.sh
+
 .PHONY: compilation-check
 compilation-check: core-deps ## Run compilation check
 	@echo "🔨 Running compilation check..."

.config/make/pre-commit.mak

@@ -7,5 +7,5 @@ setup-hooks: ## Set up git hooks
 	@echo "✅ Git hooks setup complete!"
 
 .PHONY: pre-commit
-pre-commit: fmt clippy-check compilation-check test ## Run pre-commit checks
-	@echo "✅ All pre-commit checks passed!"
+pre-commit: fmt unsafe-code-check clippy-check compilation-check test ## Run pre-commit checks
+	@echo "✅ All pre-commit checks passed!"

.github/workflows/ci.yml

@@ -130,6 +130,9 @@ jobs:
       - name: Check code formatting
         run: cargo fmt --all --check
 
+      - name: Check unsafe code allowances
+        run: ./scripts/check_unsafe_code_allowances.sh
+
       - name: Run clippy lints
         run: cargo clippy --all-targets --all-features -- -D warnings
 

README.md

@@ -151,6 +151,7 @@ Notes:
 - `RUSTFS_NOTIFY_ENABLE=true` enables the global notify module switch.
 - For ARN `arn:rustfs:sqs::primary:webhook`, use instance-scoped env vars with `_PRIMARY`.
 - If queue dir is omitted, default is `/opt/rustfs/events`; ensure it is writable by the container runtime user.
+- `RUSTFS_NOTIFY_WEBHOOK_SKIP_TLS_VERIFY_PRIMARY` defaults to `false`; enabling it skips webhook TLS certificate verification, allows MITM attacks, and emits a startup warning. Prefer `RUSTFS_NOTIFY_WEBHOOK_CLIENT_CA_PRIMARY` for private CAs.
 
 **NOTE**: We recommend reviewing the `docker-compose.yml` file before running. It defines several services including Grafana, Prometheus, and Jaeger, which are helpful for RustFS observability. If you wish to start Redis or Nginx containers, you can specify the corresponding profiles.
 

crates/ecstore/src/disk/local.rs

@@ -2160,6 +2160,8 @@ impl DiskAPI for LocalDisk {
 
     /// Zero-copy file read using memory mapping (Unix) or efficient read (non-Unix).
     /// Returns Bytes that can be shared without copying.
+    // SAFETY: Unix unsafe calls in this function only query page size and mmap
+    // a read-only file region after bounds and alignment are validated.
     #[allow(unsafe_code)]
     #[tracing::instrument(level = "debug", skip(self))]
     async fn read_file_zero_copy(&self, volume: &str, path: &str, offset: usize, length: usize) -> Result<Bytes> {
@@ -2212,6 +2214,8 @@ impl DiskAPI for LocalDisk {
                 // mmap offsets on Unix must be page-size aligned. Align the
                 // mapping down to the nearest page boundary, then slice out the
                 // originally requested logical range.
+                // SAFETY: `sysconf(_SC_PAGESIZE)` has no pointer arguments and
+                // only queries process-global OS configuration.
                 let page_size = unsafe { libc::sysconf(libc::_SC_PAGESIZE) };
                 if page_size <= 0 {
                     return Err(DiskError::other("failed to determine system page size"));

crates/io-core/src/pool.rs

@@ -443,11 +443,13 @@ impl PoolTier {
 }
 
 impl Drop for PooledBuffer {
+    // SAFETY: Drop has exclusive access to `self`; taking the `ManuallyDrop`
+    // buffer moves it exactly once into the pool when a tier still owns it.
     #[allow(unsafe_code)]
     fn drop(&mut self) {
         // Return buffer to pool if tier reference exists
         if let Some(ref tier) = self.tier {
-            // Safety: We're in drop(), so this is the last use of the buffer
+            // SAFETY: We're in drop(), so this is the last use of the buffer
             // ManuallyDrop allows us to take the value without running BytesMut's drop
             let buffer = unsafe { ManuallyDrop::take(&mut self.buffer) };
             tier.return_buffer(buffer);

crates/io-core/src/reader.rs

@@ -119,6 +119,9 @@ impl ZeroCopyObjectReader {
     /// let reader = ZeroCopyObjectReader::from_file_mmap_path("large_file.bin", 0, 1024).await?;
     /// ```
     #[cfg(unix)]
+    // SAFETY: The mmap is created from a read-only file handle for the
+    // caller-provided range, then copied into owned `Bytes` before the file and
+    // mapping are dropped.
     #[allow(unsafe_code)]
     pub async fn from_file_mmap_path(path: &std::path::Path, offset: u64, size: usize) -> Result<Self, ZeroCopyReadError> {
         use memmap2::MmapOptions;
@@ -130,7 +133,8 @@ impl ZeroCopyObjectReader {
             // Open the file in sync context
             let std_file = std::fs::File::open(&path).map_err(|e| ZeroCopyReadError::Io(e.to_string()))?;
 
-            // Create memory map
+            // SAFETY: `std_file` remains open while the mapping is created and
+            // copied, and the mapped bytes are not exposed beyond this closure.
             let mmap = unsafe { MmapOptions::new().offset(offset).len(size).map(&std_file) }
                 .map_err(|e| ZeroCopyReadError::Mmap(e.to_string()))?;
 

crates/keystone/README.md

@@ -82,6 +82,10 @@ export RUSTFS_KEYSTONE_CACHE_SIZE=10000
 export RUSTFS_KEYSTONE_CACHE_TTL=300
 ```
 
+TLS certificate verification is enabled by default. Set
+`RUSTFS_KEYSTONE_VERIFY_SSL=false` only for an explicitly trusted hop; it allows
+MITM attacks against the Keystone connection and emits a startup warning.
+
 ## API Documentation
 
 ### KeystoneClient
@@ -628,8 +632,8 @@ time curl -X GET http://localhost:9000/ \
 - Verify token format is correct (no newlines, extra spaces)
 
 **Issue: "SSL verification failed"**
-- If using self-signed certificates, set `RUSTFS_KEYSTONE_VERIFY_SSL=false`
-- Or install Keystone's CA certificate in system trust store
+- Prefer installing Keystone's CA certificate in the system trust store
+- If using a trusted non-production hop, set `RUSTFS_KEYSTONE_VERIFY_SSL=false`; this allows MITM attacks and emits a startup warning
 
 **Issue: Slow performance**
 - Increase cache size: `RUSTFS_KEYSTONE_CACHE_SIZE=50000`

crates/keystone/src/client.rs

@@ -58,6 +58,13 @@ impl KeystoneClient {
         admin_domain: String,
         verify_ssl: bool,
     ) -> Self {
+        if !verify_ssl {
+            warn!(
+                "Keystone client for '{}' is configured to skip TLS certificate verification. This permits MITM attacks and should not be used in production.",
+                auth_url
+            );
+        }
+
         let client = Client::builder()
             .danger_accept_invalid_certs(!verify_ssl)
             .timeout(std::time::Duration::from_secs(30))

crates/protos/src/lib.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// SAFETY: `generated` is prost/tonic-generated protocol code. The allowance is
+// scoped to that module so generated internals do not relax lints elsewhere.
 #[allow(unsafe_code)]
 mod generated;
 

crates/utils/src/envs.rs

@@ -668,7 +668,7 @@ pub fn apply_external_env_compat() -> ExternalEnvCompatReport {
     let report = build_external_env_compat_report();
     for (source_key, rustfs_key) in &report.mapped_pairs {
         if let Ok(value) = env::var(source_key) {
-            // Safety: this helper is intended for early startup bootstrap
+            // SAFETY: this helper is intended for early startup bootstrap
             // before any background threads are created.
             unsafe {
                 env::set_var(rustfs_key, value);

crates/utils/src/os/windows.rs

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#![allow(unsafe_code)] // TODO: audit unsafe code
-
 use crate::os::{DiskInfo, IOStats};
 use std::io::Error;
 use std::path::Path;
@@ -21,6 +19,9 @@ use windows::Win32::Foundation::MAX_PATH;
 use windows::Win32::Storage::FileSystem::{GetDiskFreeSpaceExW, GetDiskFreeSpaceW, GetVolumeInformationW, GetVolumePathNameW};
 
 /// Returns total and free bytes available in a directory, e.g. `C:\`.
+// SAFETY: Windows API calls receive null-terminated UTF-16 paths and valid
+// pointers to initialized stack output variables.
+#[allow(unsafe_code)]
 pub fn get_info(p: impl AsRef<Path>) -> std::io::Result<DiskInfo> {
     let path_wide = to_wide_path(p.as_ref());
 
@@ -81,6 +82,9 @@ pub fn get_info(p: impl AsRef<Path>) -> std::io::Result<DiskInfo> {
     })
 }
 
+// SAFETY: Windows volume APIs receive null-terminated UTF-16 paths and fixed
+// stack buffers sized for the documented MAX_PATH outputs used here.
+#[allow(unsafe_code)]
 fn get_windows_fs_type(p: &[u16]) -> std::io::Result<String> {
     let path = get_volume_name(p)?;
 
@@ -109,6 +113,9 @@ fn get_windows_fs_type(p: &[u16]) -> std::io::Result<String> {
     Ok(utf16_to_string(&file_system_name_buffer))
 }
 
+// SAFETY: `v` is a null-terminated UTF-16 path and `volume_name_buffer` is a
+// writable MAX_PATH-sized stack buffer for the returned volume path.
+#[allow(unsafe_code)]
 fn get_volume_name(v: &[u16]) -> std::io::Result<Vec<u16>> {
     let mut volume_name_buffer = [0u16; MAX_PATH as usize];
 

rustfs/src/admin/router.rs

@@ -651,6 +651,10 @@ fn build_object_lambda_http_client(config: &ObjectLambdaWebhookConfig) -> S3Resu
     }
 
     if config.skip_tls_verify {
+        warn!(
+            "Object Lambda webhook target '{}' is configured to skip TLS certificate verification. This permits MITM attacks and should not be used in production.",
+            config.endpoint
+        );
         builder = builder.danger_accept_invalid_certs(true);
     } else if !config.client_ca.is_empty() {
         let ca_pem = std::fs::read(&config.client_ca)

rustfs/src/allocator_reclaim.rs

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#![allow(unsafe_code)]
-
 use metrics::{counter, gauge, histogram};
 use std::time::Duration;
 use tokio_util::sync::CancellationToken;
@@ -91,6 +89,7 @@ fn reclaimable_work_snapshot() -> ReclaimableWorkSnapshot {
     not(target_os = "windows"),
     not(all(target_os = "linux", target_env = "gnu", target_arch = "x86_64"))
 ))]
+#[allow(unsafe_code)]
 fn collect_allocator_memory(force: bool) -> Result<(), String> {
     // SAFETY: `mi_collect` is provided by the active global allocator backend
     // on this target family. It is explicitly intended to reclaim retained

rustfs/src/config/config_test.rs

@@ -28,6 +28,8 @@ mod tests {
     /// # Safety
     /// This function uses unsafe env::set_var and env::remove_var.
     /// Tests using this helper must be marked with #[serial] to avoid race conditions.
+    // SAFETY: This helper mutates process environment only inside serial tests
+    // and restores the variable before returning or resuming a panic.
     #[allow(unsafe_code)]
     fn with_env_var<F>(key: &str, value: &str, test_fn: F)
     where
@@ -366,7 +368,6 @@ mod tests {
     /// Uses #[serial] to avoid concurrent env var modifications.
     #[test]
     #[serial]
-    #[allow(unsafe_code)]
     fn test_rustfs_volumes_env_variable() {
         // Test case 1: Single volume via environment variable
         with_env_var("RUSTFS_VOLUMES", "/data/vol1", || {
@@ -511,7 +512,6 @@ mod tests {
     /// which means paths with spaces are NOT supported.
     #[test]
     #[serial]
-    #[allow(unsafe_code)]
     fn test_volumes_boundary_cases() {
         // Test case 1: Paths with spaces are not properly supported (known limitation)
         // This test documents the current behavior - space-separated paths will be split
@@ -670,7 +670,6 @@ mod tests {
 
     #[test]
     #[serial]
-    #[allow(unsafe_code)]
     fn test_access_key_arguments_mutually_exclusive_env_var() {
         // Test that env var args configuration fails on conflict
         with_env_var("RUSTFS_VOLUMES", "/data/my disk/vol1", || {
@@ -710,7 +709,6 @@ mod tests {
 
     #[test]
     #[serial]
-    #[allow(unsafe_code)]
     fn test_secret_key_arguments_mutually_exclusive_env_var() {
         // Test that env var args configuration fails on conflict
         with_env_var("RUSTFS_VOLUMES", "/data/my disk/vol1", || {

rustfs/src/profiling/allocator.rs

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#![allow(unsafe_code)]
-
 use backtrace::Backtrace;
 use pprof::protos::Message;
 use rand::RngExt;
@@ -343,6 +341,11 @@ fn handle_dealloc_sampling(ptr: *mut u8) {
     }
 }
 
+// SAFETY: This allocator wrapper preserves the `GlobalAlloc` contract by
+// delegating all allocation operations to `inner` with the exact caller-provided
+// layouts and pointers. Sampling records metadata only and does not take
+// ownership of allocation memory.
+#[allow(unsafe_code)]
 unsafe impl<A: GlobalAlloc> GlobalAlloc for TracingAllocator<A> {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         // SAFETY: Delegating to inner allocator.
@@ -376,6 +379,10 @@ unsafe impl<A: GlobalAlloc> GlobalAlloc for TracingAllocator<A> {
 }
 
 #[cfg(test)]
+// SAFETY: Tests call the unsafe `GlobalAlloc` methods with layouts created by
+// `Layout::from_size_align` and deallocate each returned pointer with the same
+// layout.
+#[allow(unsafe_code)]
 mod tests {
     use super::*;
     use serial_test::serial;

rustfs/src/server/http.rs

@@ -1021,6 +1021,9 @@ fn get_listen_backlog() -> i32 {
 // For macOS and BSD variants use the syscall way of getting the connection queue length.
 // NetBSD has no somaxconn-like kernel state.
 #[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "openbsd"))]
+// SAFETY: The only unsafe operation in this function is `libc::sysctl`, called
+// with kernel MIB arrays selected by target OS, a valid output buffer, and no
+// input buffer.
 #[allow(unsafe_code)]
 fn get_listen_backlog() -> i32 {
     const DEFAULT_BACKLOG: i32 = 1024;
@@ -1032,6 +1035,8 @@ fn get_listen_backlog() -> i32 {
     let mut buf = [0; 1];
     let mut buf_len = size_of_val(&buf);
 
+    // SAFETY: `name` points to the target OS MIB, `buf` is a valid writable
+    // output buffer, `buf_len` points to its size, and no input buffer is used.
     if unsafe {
         libc::sysctl(
             name.as_mut_ptr(),

scripts/check_unsafe_code_allowances.sh

@@ -0,0 +1,20 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ROOT_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)"
+
+cd "$ROOT_DIR"
+
+status=0
+
+while IFS=: read -r file line _; do
+  start=$((line > 3 ? line - 3 : 1))
+  end=$((line + 6))
+  if ! sed -n "${start},${end}p" "$file" | rg -q "SAFETY:"; then
+    printf '%s:%s: unsafe_code allowance must have a nearby SAFETY comment\n' "$file" "$line" >&2
+    status=1
+  fi
+done < <(rg -n '^[[:space:]]*#!?\[allow\([^]]*\bunsafe_code\b[^]]*\)\]' --glob '*.rs' .)
+
+exit "$status"