Skip to content

Commit 874bca2

Browse files
authored
fix: repair legacy database permissions (#5)
* fix(store): normalize owned legacy database modes * fix(store): harden legacy database normalization Split private_io into a Unix module and dedicated test files, and add path normalization so relative and macOS /tmp,/var-aliased database paths resolve consistently before descriptor validation. Descriptor validation is now nonblocking and enforces mode, owner, and trusted ancestor policy on existing legacy databases. Move the file-store integration tests into store_file_tests.rs and the private_io unit tests into private_io_tests.rs to keep modules focused.
1 parent 8777ff4 commit 874bca2

7 files changed

Lines changed: 696 additions & 313 deletions

File tree

crates/threads-store/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ mod migrations;
1212
mod private_io;
1313
mod query;
1414
mod store;
15+
#[cfg(all(test, unix))]
16+
#[path = "store_file_tests.rs"]
17+
mod store_file_tests;
1518
#[cfg(test)]
1619
mod tests;
1720

Lines changed: 33 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -1,162 +1,48 @@
1-
use std::{io, path::Path};
1+
use std::path::{Path, PathBuf};
22

33
use crate::{Result, StoreError};
44

5-
pub(crate) fn prepare_database_path(path: &Path) -> Result<()> {
6-
#[cfg(unix)]
7-
{
8-
create_safe_parent(path)?;
9-
create_or_validate_database(path)
10-
.and_then(|()| create_or_validate_database(&sqlite_sidecar(path, "-wal")))
11-
.and_then(|()| validate_existing_database(&sqlite_sidecar(path, "-shm")))
12-
}
13-
14-
#[cfg(not(unix))]
15-
{
16-
let _ = path;
17-
Ok(())
18-
}
19-
}
20-
215
#[cfg(unix)]
22-
fn create_safe_parent(path: &Path) -> Result<()> {
23-
use std::{fs, os::unix::fs::DirBuilderExt};
6+
#[path = "private_io_unix.rs"]
7+
mod unix;
248

25-
let parent = path
26-
.parent()
27-
.filter(|parent| !parent.as_os_str().is_empty())
28-
.unwrap_or_else(|| Path::new("."));
29-
let mut missing = Vec::new();
30-
let mut current = parent;
9+
#[cfg(unix)]
10+
pub(crate) use unix::prepare_database_path;
3111

32-
loop {
33-
match fs::symlink_metadata(current) {
34-
Ok(metadata) => {
35-
validate_parent(current, &metadata)?;
36-
break;
37-
}
38-
Err(error) if error.kind() == io::ErrorKind::NotFound => {
39-
missing.push(current.to_path_buf());
40-
current = current.parent().ok_or_else(|| {
41-
StoreError::Io(io::Error::new(
42-
io::ErrorKind::NotFound,
43-
format!("no existing parent for {}", path.display()),
44-
))
45-
})?;
46-
}
47-
Err(error) => return Err(StoreError::Io(error)),
12+
#[cfg(all(test, unix))]
13+
use unix::{
14+
effective_user_id, has_expected_owner, is_trusted_ancestor, normalize_existing_database,
15+
open_existing_database, sqlite_sidecar,
16+
};
17+
18+
pub(crate) fn normalize_database_path(path: &Path) -> Result<PathBuf> {
19+
let absolute = if path.is_absolute() {
20+
path.to_path_buf()
21+
} else {
22+
std::env::current_dir().map_err(StoreError::Io)?.join(path)
23+
};
24+
Ok(normalize_macos_system_alias(&absolute))
25+
}
26+
27+
fn normalize_macos_system_alias(path: &Path) -> PathBuf {
28+
#[cfg(target_os = "macos")]
29+
{
30+
if let Ok(suffix) = path.strip_prefix("/var") {
31+
return Path::new("/private/var").join(suffix);
4832
}
49-
}
50-
51-
for directory in missing.iter().rev() {
52-
let mut builder = fs::DirBuilder::new();
53-
builder.mode(0o700);
54-
match builder.create(directory) {
55-
Ok(()) => {}
56-
Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {}
57-
Err(error) => return Err(StoreError::Io(error)),
33+
if let Ok(suffix) = path.strip_prefix("/tmp") {
34+
return Path::new("/private/tmp").join(suffix);
5835
}
59-
validate_parent(
60-
directory,
61-
&fs::symlink_metadata(directory).map_err(StoreError::Io)?,
62-
)?;
63-
}
64-
65-
Ok(())
66-
}
67-
68-
#[cfg(unix)]
69-
fn create_or_validate_database(path: &Path) -> Result<()> {
70-
use std::{fs, os::unix::fs::OpenOptionsExt};
71-
72-
match fs::symlink_metadata(path) {
73-
Ok(metadata) => validate_database(path, &metadata),
74-
Err(error) if error.kind() == io::ErrorKind::NotFound => fs::OpenOptions::new()
75-
.write(true)
76-
.create_new(true)
77-
.mode(0o600)
78-
.custom_flags(libc::O_NOFOLLOW)
79-
.open(path)
80-
.map(|_| ())
81-
.map_err(StoreError::Io),
82-
Err(error) => Err(StoreError::Io(error)),
8336
}
37+
path.to_path_buf()
8438
}
8539

86-
#[cfg(unix)]
87-
fn validate_existing_database(path: &Path) -> Result<()> {
88-
match std::fs::symlink_metadata(path) {
89-
Ok(metadata) => validate_database(path, &metadata),
90-
Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()),
91-
Err(error) => Err(StoreError::Io(error)),
92-
}
93-
}
94-
95-
#[cfg(unix)]
96-
fn sqlite_sidecar(path: &Path, suffix: &str) -> std::path::PathBuf {
97-
let mut sidecar = path.as_os_str().to_os_string();
98-
sidecar.push(suffix);
99-
std::path::PathBuf::from(sidecar)
100-
}
101-
102-
#[cfg(unix)]
103-
fn validate_parent(path: &Path, metadata: &std::fs::Metadata) -> Result<()> {
104-
use std::os::unix::fs::PermissionsExt;
105-
106-
if metadata.file_type().is_symlink() {
107-
return unsafe_path(path, "parent is a symlink");
108-
}
109-
if !metadata.is_dir() {
110-
return unsafe_path(path, "parent is not a directory");
111-
}
112-
if metadata.permissions().mode() & 0o022 != 0 {
113-
return unsafe_path(path, "parent is group or world writable");
114-
}
115-
Ok(())
116-
}
117-
118-
#[cfg(unix)]
119-
fn validate_database(path: &Path, metadata: &std::fs::Metadata) -> Result<()> {
120-
use std::os::unix::fs::PermissionsExt;
121-
122-
if metadata.file_type().is_symlink() {
123-
return unsafe_path(path, "database is a symlink");
124-
}
125-
if !metadata.is_file() {
126-
return unsafe_path(path, "database is not a regular file");
127-
}
128-
if metadata.permissions().mode() & 0o077 != 0 {
129-
return unsafe_path(path, "database is readable or writable by group or world");
130-
}
40+
#[cfg(not(unix))]
41+
pub(crate) fn prepare_database_path(path: &Path) -> Result<()> {
42+
let _ = path;
13143
Ok(())
13244
}
13345

134-
#[cfg(unix)]
135-
fn unsafe_path(path: &Path, reason: &str) -> Result<()> {
136-
Err(StoreError::Io(io::Error::new(
137-
io::ErrorKind::PermissionDenied,
138-
format!("unsafe path {}: {reason}", path.display()),
139-
)))
140-
}
141-
14246
#[cfg(all(test, unix))]
143-
mod tests {
144-
use std::{fs, os::unix::fs::PermissionsExt};
145-
146-
use tempfile::TempDir;
147-
148-
use super::prepare_database_path;
149-
150-
#[test]
151-
fn newly_created_database_is_private_before_sqlite_opens() {
152-
let temp = TempDir::new().unwrap();
153-
let path = temp.path().join("store.db");
154-
155-
prepare_database_path(&path).unwrap();
156-
157-
assert_eq!(
158-
fs::metadata(path).unwrap().permissions().mode() & 0o777,
159-
0o600
160-
);
161-
}
162-
}
47+
#[path = "private_io_tests.rs"]
48+
mod tests;

0 commit comments

Comments
 (0)