Narrow a file that already existed, not just one being created

`OpenOptions::mode` applies only when the file is created, so opening one
that already exists keeps whatever mode it had. For `append_file` that is
the normal case rather than the odd one -- every write to a transcript
after the first -- and for `create_file` it happens on every rewrite. A
file made wrong by an older version or by hand would have stayed
world-readable for as long as it was only ever appended to.

This is the same reasoning as `create_dir`'s second call, which was
already here and already commented. Missing it on the file paths was the
kind of gap that only shows when the two are read side by side, which is
an argument for this crate existing rather than against it.

Set through the open handle rather than the path, so nothing can swap
what is at that path between the open and the chmod.

Caught by ai-app's session reviewing the module I had taken from their
code, which is the review this repo is supposed to make possible.
This commit is contained in:
iris committed 2026-08-28 13:30:41 -04:00
1 parent 7651d491ac
commit 861e6a329b
1 file changed
+64 -13
+64 -13
View File
@@ -46,13 +46,16 @@ pub fn write_file(path: &Path, contents: &[u8]) -> Result<()> {
/// Opens `path` for writing, owner-readable only, truncating what is
/// there. For a caller that streams rather than holding the whole body.
pub fn create_file(path: &Path) -> Result<File> {
std::fs::OpenOptions::new()
.write(true)
.create(true)
.truncate(true)
.mode(0o600)
.open(path)
.with_context(|| format!("write {}", path.display()))
restrict(
path,
std::fs::OpenOptions::new()
.write(true)
.create(true)
.truncate(true)
.mode(0o600)
.open(path)
.with_context(|| format!("write {}", path.display()))?,
)
}
/// Opens `path` for appending, owner-readable only, creating it if needed.
@@ -61,12 +64,33 @@ pub fn create_file(path: &Path) -> Result<File> {
/// truncated by being opened, and the two differ by one flag that is easy
/// to get wrong in a hurry.
pub fn append_file(path: &Path) -> Result<File> {
std::fs::OpenOptions::new()
.append(true)
.create(true)
.mode(0o600)
.open(path)
.with_context(|| format!("append to {}", path.display()))
restrict(
path,
std::fs::OpenOptions::new()
.append(true)
.create(true)
.mode(0o600)
.open(path)
.with_context(|| format!("append to {}", path.display()))?,
)
}
/// Narrows an already-open file to owner-only.
///
/// `OpenOptions::mode` applies **only when the file is created**, so
/// opening one that already exists silently keeps whatever mode it had --
/// which for an append helper is the normal case rather than the odd one,
/// and for a truncating one happens on every rewrite after the first.
/// Setting it through the handle rather than the path closes the window
/// where something could swap the path between the two.
///
/// The same reasoning as [`create_dir`]'s second call, and it was missed
/// here first: a file made wrong by an older version, or by hand, would
/// otherwise stay wrong for as long as it is only ever appended to.
fn restrict(path: &Path, file: File) -> Result<File> {
file.set_permissions(std::fs::Permissions::from_mode(0o600))
.with_context(|| format!("restrict {}", path.display()))?;
Ok(file)
}
#[cfg(test)]
@@ -95,6 +119,33 @@ mod tests {
);
}
/// The case `OpenOptions::mode` cannot cover, because it applies only
/// at creation: a file that already exists, made wrong earlier, and
/// opened again. An append-only transcript hits this on every write
/// after the first.
#[test]
fn an_existing_file_made_wrong_is_narrowed_on_open() {
let dir = tempfile::tempdir().expect("tempdir");
create_dir(dir.path()).expect("create_dir");
for (name, open) in [
("appended", append_file as fn(&Path) -> Result<File>),
("rewritten", create_file as fn(&Path) -> Result<File>),
] {
let path = dir.path().join(name);
std::fs::write(&path, b"made by an older version").expect("write");
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).expect("chmod");
assert_eq!(mode_of(&path), 0o644, "precondition");
open(&path).expect("open");
assert_eq!(
mode_of(&path),
0o600,
"{name} kept a mode somebody else could read"
);
}
}
#[test]
fn files_are_owner_only_from_the_moment_they_exist() {
let dir = tempfile::tempdir().expect("tempdir");