From d469897d66e91889eccbff68ed2bd303b33d96b6 Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Tue, 10 Nov 2026 20:34:41 +0100 Subject: [PATCH 1/5] fix: Write secret files without following symlinks Processes in the Pod can write to the volume, because the kubelet applies the fsGroup with group write permissions, so any entry in it may have been replaced since the last publish. Secret files and their directories are now opened relative to their parent without following symlinks, and data is only written into regular files. --- CHANGELOG.md | 2 + Cargo.lock | 1 + Cargo.nix | 5 + Cargo.toml | 1 + rust/operator-binary/Cargo.toml | 1 + rust/operator-binary/src/csi_server/node.rs | 274 ++++++++++++++------ typos.toml | 2 + 7 files changed, 211 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f13053f..2149a0e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ All notable changes to this project will be documented in this file. - In privileged mode, `NodePublishVolume` reuses an existing tmpfs mount instead of mounting another one on top, so a repeated publish (for example after a kubelet restart) no longer stacks mounts, which kept Pods in `Terminating` for minutes ([#770]). - `NodePublishVolume` truncates the secret files it rewrites, so a repeated publish no longer leaves trailing bytes of a longer previous file behind ([#770]). - A repeated `NodePublishVolume` (for example after a kubelet restart) keeps the published secret instead of issuing a new one, so the expiry annotation keeps matching the secret the Pod uses ([#773]). +- `NodePublishVolume` no longer follows symlinks inside the volume and only writes secret data into regular files ([#775]). - Report the certificate expiry for the `certManager` backend, so that Pods are restarted before their certificate expires and pick up the renewed one. Previously no expiry was reported at all, so a Pod kept the certificate it was given at startup and eventually ran on an expired one @@ -44,6 +45,7 @@ All notable changes to this project will be documented in this file. [#766]: https://github.com/stackabletech/secret-operator/pull/766 [#770]: https://github.com/stackabletech/secret-operator/pull/770 [#773]: https://github.com/stackabletech/secret-operator/pull/773 +[#775]: https://github.com/stackabletech/secret-operator/pull/775 ## [26.7.0] - 2026-07-21 diff --git a/Cargo.lock b/Cargo.lock index f442d6c1..731e7ab7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3921,6 +3921,7 @@ dependencies = [ "prost 0.14.4", "prost-types", "rand 0.10.3", + "rustix", "serde", "serde_json", "snafu 0.9.2", diff --git a/Cargo.nix b/Cargo.nix index 077068dd..36ee0138 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -13646,6 +13646,11 @@ rec { name = "rand"; packageId = "rand 0.10.3"; } + { + name = "rustix"; + packageId = "rustix"; + features = [ "fs" ]; + } { name = "serde"; packageId = "serde"; diff --git a/Cargo.toml b/Cargo.toml index 658a544d..e545726e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ pin-project = "1.1" prost = "0.14" prost-types = "0.14" rand = "0.10" +rustix = { version = "1.1", features = ["fs"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" snafu = "0.9" diff --git a/rust/operator-binary/Cargo.toml b/rust/operator-binary/Cargo.toml index aefe94b4..e2751529 100644 --- a/rust/operator-binary/Cargo.toml +++ b/rust/operator-binary/Cargo.toml @@ -44,6 +44,7 @@ tonic-reflection.workspace = true tracing.workspace = true uuid.workspace = true rand.workspace = true +rustix.workspace = true yaml_serde.workspace = true [dev-dependencies] diff --git a/rust/operator-binary/src/csi_server/node.rs b/rust/operator-binary/src/csi_server/node.rs index 3b2567d6..393ed4bf 100644 --- a/rust/operator-binary/src/csi_server/node.rs +++ b/rust/operator-binary/src/csi_server/node.rs @@ -1,12 +1,17 @@ use std::{ - fs::Permissions, + fs::{File, Permissions}, + io::Write, os::unix::prelude::{MetadataExt, PermissionsExt}, path::{Component, Path, PathBuf}, }; use openssl::sha::Sha256; +use rustix::{ + fs::{CWD, FileType, Mode, OFlags, fstat, mkdirat, openat}, + io::Errno, +}; use serde::{Deserialize, de::IntoDeserializer}; -use snafu::{ResultExt, Snafu, ensure}; +use snafu::{OptionExt, ResultExt, Snafu, ensure}; use stackable_operator::{ builder::meta::ObjectMetaBuilder, k8s_openapi::api::core::v1::Pod, @@ -14,10 +19,6 @@ use stackable_operator::{ kvp::{AnnotationError, Annotations}, }; use sys_mount::{Mount, MountFlags, UnmountFlags, unmount}; -use tokio::{ - fs::{OpenOptions, create_dir_all}, - io::AsyncWriteExt, -}; use tonic::{Request, Response, Status}; use super::{controller::TOPOLOGY_NODE, log_if_endpoint_error}; @@ -108,6 +109,9 @@ enum PublishError { #[snafu(display("provisioned marker {path:?} is not a regular file"))] InvalidProvisionedMarker { path: PathBuf }, + #[snafu(display("secret file {path:?} exists but is not a regular file"))] + NotRegularFile { path: PathBuf }, + #[snafu(display("file path {path:?} must only contain normal components"))] InvalidComponents { path: PathBuf }, @@ -145,6 +149,7 @@ impl From for Status { PublishError::WriteFile { .. } => Status::unavailable(full_msg), PublishError::CheckProvisionedMarker { .. } => Status::unavailable(full_msg), PublishError::InvalidProvisionedMarker { .. } => Status::failed_precondition(full_msg), + PublishError::NotRegularFile { .. } => Status::failed_precondition(full_msg), PublishError::InvalidComponents { .. } => Status::unavailable(full_msg), PublishError::InvalidAbsolutePath { .. } => Status::unavailable(full_msg), PublishError::TagPod { .. } => Status::unavailable(full_msg), @@ -255,68 +260,12 @@ impl SecretProvisionerNode { .. } = selector; - let create_secret = { - let mut opts = OpenOptions::new(); - opts.create(true) - .write(true) - // A repeated publish rewrites existing files, and regenerated keys and - // certificates vary in length. Without truncating, a shorter file keeps the - // trailing bytes of the previous one. Not good. So we truncate. - .truncate(true) - // User: root/secret-operator - // Group: Controlled by Pod.securityContext.fsGroup, the actual application - // (when running as unprivileged user) - .mode(0o640); - opts - }; - for (k, v) in data .data .into_files(format, names, compat, provision_parts) .context(publish_error::FormatDataSnafu)? { - // The following few lines of code do some basic checks against - // unwanted path traversals. In the future, we want to leverage - // capability based filesystem operations (openat) to prevent these - // traversals. - - // First, let's turn the (potentially custom) file path into a path. - let file_path = PathBuf::from(k); - - // Next, ensure the path is not absolute (does not contain root), - // because joining an absolute path with a different path will - // replace the exiting path entirely. - ensure!( - !file_path.has_root(), - publish_error::InvalidAbsolutePathSnafu { path: &file_path } - ); - - // Ensure that the file path only contains normal components. This - // prevents any path traversals up the path using '..'. - ensure!( - file_path - .components() - .all(|c| matches!(c, Component::Normal(_))), - publish_error::InvalidComponentsSnafu { path: &file_path } - ); - - // Now, we can join the base and file path - let item_path = target_path.join(file_path); - - if let Some(item_path_parent) = item_path.parent() { - create_dir_all(item_path_parent) - .await - .context(publish_error::CreateDirSnafu { - path: item_path_parent, - })?; - } - create_secret - .open(&item_path) - .await - .context(publish_error::CreateFileSnafu { path: &item_path })? - .write_all(&v) - .await - .context(publish_error::WriteFileSnafu { path: item_path })?; + write_secret_file(target_path, Path::new(&k), &v)?; } Ok(()) } @@ -447,17 +396,107 @@ async fn is_provisioned(target_path: &Path) -> Result { /// Must be called after all secret files are written. async fn mark_provisioned(target_path: &Path) -> Result<(), PublishError> { - let path = target_path.join(PROVISIONED_MARKER_FILE_NAME); - OpenOptions::new() - .create(true) - .write(true) - .truncate(true) - .mode(0o640) - .custom_flags(libc::O_NOFOLLOW) - .open(&path) - .await - .context(publish_error::CreateFileSnafu { path })?; - Ok(()) + write_secret_file(target_path, Path::new(PROVISIONED_MARKER_FILE_NAME), &[]) +} + +/// Writes `contents` to `relative_path` beneath `root`, creating missing directories. +/// +/// `relative_path` can be chosen by users, so it must be relative and only consist of normal +/// components, otherwise the write fails. +/// +/// The Pod can write to the volume, so since the last publish it may have replaced any entry in +/// it, for example with a symlink. This function never follows a symlink, neither for the file +/// nor for the directories leading to it. Each directory is opened through a file descriptor of +/// the directory before it rather than by path, so replacing an entry after it has been opened +/// cannot redirect the write. It only writes into regular files: a FIFO or device node in place +/// of the file is an error. +fn write_secret_file( + root: &Path, + relative_path: &Path, + contents: &[u8], +) -> Result<(), PublishError> { + // Ensure that `relative_path` is indeed a relative path + ensure!( + !relative_path.has_root(), + publish_error::InvalidAbsolutePathSnafu { + path: relative_path + } + ); + + // Ensure that all components of the path are "normal" + // Examples for not-normal are things like "." and ".." + ensure!( + relative_path + .components() + .all(|c| matches!(c, Component::Normal(_))), + publish_error::InvalidComponentsSnafu { + path: relative_path + } + ); + + // This creates the directory structure needed to get to the relative directory + // making sure to not follow symlinks and so on + let directory_flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC; + let file_name = relative_path + .file_name() + .context(publish_error::InvalidComponentsSnafu { + path: relative_path, + })?; + + let mut path = root.to_path_buf(); + let mut directory = openat(CWD, root, directory_flags, Mode::empty()) + .map_err(std::io::Error::from) + .context(publish_error::CreateDirSnafu { path: &path })?; + for component in relative_path + .parent() + .into_iter() + .flat_map(Path::components) + { + let name = component.as_os_str(); + path.push(name); + match mkdirat(&directory, name, Mode::from_raw_mode(0o777)) { + Ok(()) | Err(Errno::EXIST) => {} + Err(err) => { + return Err(std::io::Error::from(err)) + .context(publish_error::CreateDirSnafu { path }); + } + } + directory = openat(&directory, name, directory_flags, Mode::empty()) + .map_err(std::io::Error::from) + .context(publish_error::CreateDirSnafu { path: &path })?; + } + + // And now finally we create the actual file from `contents` + path.push(file_name); + let file = openat( + &directory, + file_name, + OFlags::WRONLY + | OFlags::CREATE + // A repeated publish rewrites existing files, and regenerated keys and certificates + // vary in length, so a shorter file would keep trailing bytes of the previous one. + | OFlags::TRUNC + | OFlags::NOFOLLOW + | OFlags::NONBLOCK + | OFlags::CLOEXEC, + // User: root/secret-operator + // Group: Controlled by Pod.securityContext.fsGroup, the actual application + // (when running as unprivileged user) + Mode::from_raw_mode(0o640), + ) + .map_err(std::io::Error::from) + .context(publish_error::CreateFileSnafu { path: &path })?; + let file_type = fstat(&file) + .map(|stat| FileType::from_raw_mode(stat.st_mode)) + .map_err(std::io::Error::from) + .context(publish_error::CreateFileSnafu { path: &path })?; + ensure!( + file_type == FileType::RegularFile, + publish_error::NotRegularFileSnafu { path } + ); + File::from(file) + .write_all(contents) + .context(publish_error::WriteFileSnafu { path }) } // Most of the services are not yet implemented, most of them will never be, because they are @@ -600,7 +639,12 @@ impl Node for SecretProvisionerNode { mod tests { use std::path::Path; - use super::{PROVISIONED_MARKER_FILE_NAME, is_mount_point, is_provisioned, mark_provisioned}; + use rustix::fs::{CWD, FileType, Mode, mknodat}; + + use super::{ + PROVISIONED_MARKER_FILE_NAME, PublishError, is_mount_point, is_provisioned, + mark_provisioned, write_secret_file, + }; #[tokio::test] async fn plain_directory_is_not_mount_point() { @@ -664,4 +708,84 @@ mod tests { assert!(mark_provisioned(volume.path()).await.is_err()); assert!(!target.exists()); } + + #[test] + fn writes_nested_secret_file() { + let volume = tempfile::tempdir().expect("temporary directory can be created"); + write_secret_file(volume.path(), Path::new("nested/tls.crt"), b"certificate") + .expect("secret file can be written"); + let contents = + std::fs::read(volume.path().join("nested/tls.crt")).expect("secret file is readable"); + assert_eq!(contents, b"certificate"); + } + + #[test] + fn rewriting_truncates_secret_file() { + let volume = tempfile::tempdir().expect("temporary directory can be created"); + write_secret_file(volume.path(), Path::new("tls.key"), b"longer key") + .expect("secret file can be written"); + write_secret_file(volume.path(), Path::new("tls.key"), b"key") + .expect("secret file can be rewritten"); + let contents = + std::fs::read(volume.path().join("tls.key")).expect("secret file is readable"); + assert_eq!(contents, b"key"); + } + + #[test] + fn symlinked_secret_file_is_not_followed() { + let volume = tempfile::tempdir().expect("temporary directory can be created"); + let outside = tempfile::tempdir().expect("temporary directory can be created"); + let target = outside.path().join("victim"); + std::fs::write(&target, "untouched").expect("symlink target can be written"); + std::os::unix::fs::symlink(&target, volume.path().join("tls.crt")) + .expect("symlink can be created"); + + assert!(write_secret_file(volume.path(), Path::new("tls.crt"), b"certificate").is_err()); + let contents = std::fs::read_to_string(&target).expect("symlink target is readable"); + assert_eq!(contents, "untouched"); + } + + #[test] + fn symlinked_directory_is_not_followed() { + let volume = tempfile::tempdir().expect("temporary directory can be created"); + let outside = tempfile::tempdir().expect("temporary directory can be created"); + std::os::unix::fs::symlink(outside.path(), volume.path().join("nested")) + .expect("symlink can be created"); + + assert!( + write_secret_file(volume.path(), Path::new("nested/tls.crt"), b"certificate").is_err() + ); + assert!(!outside.path().join("tls.crt").exists()); + } + + #[test] + fn fifo_in_place_of_secret_file_is_rejected() { + let volume = tempfile::tempdir().expect("temporary directory can be created"); + mknodat( + CWD, + volume.path().join("tls.crt"), + FileType::Fifo, + Mode::from_raw_mode(0o600), + 0, + ) + .expect("FIFO can be created"); + + let result = write_secret_file(volume.path(), Path::new("tls.crt"), b"certificate"); + assert!(matches!( + result, + Err(PublishError::NotRegularFile { .. } | PublishError::CreateFile { .. }) + )); + } + + #[test] + fn secret_file_outside_volume_is_rejected() { + let root = tempfile::tempdir().expect("temporary directory can be created"); + let volume = root.path().join("volume"); + std::fs::create_dir(&volume).expect("volume directory can be created"); + + for path in ["../escaped", "/escaped", "nested/../../escaped"] { + assert!(write_secret_file(&volume, Path::new(path), b"certificate").is_err()); + } + assert!(!root.path().join("escaped").exists()); + } } diff --git a/typos.toml b/typos.toml index 7d0acd88..9d07e578 100644 --- a/typos.toml +++ b/typos.toml @@ -52,3 +52,5 @@ passin = "passin" tpe = "tpe" # Triple-DES EDE (encrypt-decrypt-encrypt) cipher type from the `des` crate. TdesEde3 = "TdesEde3" +# open(2) flag for write-only access, as `OFlags::WRONLY` in `rustix`. +WRONLY = "WRONLY" From 3a8771f832880383a960548d2b31677b3ce65733 Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Tue, 10 Nov 2026 22:46:05 +0100 Subject: [PATCH 2/5] chore: Bump serde-saphyr to 2.1.1 --- Cargo.lock | 6 +++--- Cargo.nix | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 731e7ab7..d262a7eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3490,12 +3490,12 @@ dependencies = [ [[package]] name = "serde-saphyr" -version = "2.1.0" +version = "2.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f4dc2714fc5632417a6e979602d7044bad8f2880c63fc7217087180acec31b6" +checksum = "d43fb82d550b07e6f929a58d7d0935182602f48ddead875f5ee313c41c8f07fe" dependencies = [ "annotate-snippets", - "base64 0.23.1", + "base64 0.22.1", "encoding_rs_io", "granit-parser", "nohash-hasher", diff --git a/Cargo.nix b/Cargo.nix index 36ee0138..1b0ded86 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -12186,10 +12186,10 @@ rec { }; "serde-saphyr" = rec { crateName = "serde-saphyr"; - version = "2.1.0"; + version = "2.1.1"; edition = "2024"; crateBin = []; - sha256 = "1diixjn80w882xrgqqqci3rdifj4f0nn0yg9lqbj8qy59xqw4k9z"; + sha256 = "1zh7iwfc84z3brgqgbfyips049hq6l4pv3d557wyc1qbalnvhgyl"; libName = "serde_saphyr"; license = "MIT OR Apache-2.0"; dependencies = [ @@ -12200,7 +12200,7 @@ rec { } { name = "base64"; - packageId = "base64 0.23.1"; + packageId = "base64 0.22.1"; optional = true; } { From da8345d61226116b05a5c91c8ded94686d6d8e8b Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Wed, 11 Nov 2026 12:04:19 +0100 Subject: [PATCH 3/5] refactor: Import snafu traits anonymously --- rust/operator-binary/src/csi_server/node.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/csi_server/node.rs b/rust/operator-binary/src/csi_server/node.rs index 393ed4bf..a743001b 100644 --- a/rust/operator-binary/src/csi_server/node.rs +++ b/rust/operator-binary/src/csi_server/node.rs @@ -11,7 +11,7 @@ use rustix::{ io::Errno, }; use serde::{Deserialize, de::IntoDeserializer}; -use snafu::{OptionExt, ResultExt, Snafu, ensure}; +use snafu::{OptionExt as _, ResultExt as _, Snafu, ensure}; use stackable_operator::{ builder::meta::ObjectMetaBuilder, k8s_openapi::api::core::v1::Pod, From 191e12758b347ab066adaba68043dd9991cc39aa Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Fri, 13 Nov 2026 14:16:12 +0100 Subject: [PATCH 4/5] fix: Address review comments on write_secret_file --- Cargo.nix | 3 +- Cargo.toml | 2 +- rust/operator-binary/src/csi_server/node.rs | 66 ++++++++++----------- 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/Cargo.nix b/Cargo.nix index 1b0ded86..fec73691 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -13649,7 +13649,8 @@ rec { { name = "rustix"; packageId = "rustix"; - features = [ "fs" ]; + usesDefaultFeatures = false; + features = [ "fs" "std" ]; } { name = "serde"; diff --git a/Cargo.toml b/Cargo.toml index e545726e..e06a896f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ pin-project = "1.1" prost = "0.14" prost-types = "0.14" rand = "0.10" -rustix = { version = "1.1", features = ["fs"] } +rustix = { version = "1.1", default-features = false, features = ["fs", "std"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" snafu = "0.9" diff --git a/rust/operator-binary/src/csi_server/node.rs b/rust/operator-binary/src/csi_server/node.rs index a743001b..ef29830c 100644 --- a/rust/operator-binary/src/csi_server/node.rs +++ b/rust/operator-binary/src/csi_server/node.rs @@ -399,9 +399,9 @@ async fn mark_provisioned(target_path: &Path) -> Result<(), PublishError> { write_secret_file(target_path, Path::new(PROVISIONED_MARKER_FILE_NAME), &[]) } -/// Writes `contents` to `relative_path` beneath `root`, creating missing directories. +/// Writes `contents` to `file_path` beneath `target_path`, creating missing directories. /// -/// `relative_path` can be chosen by users, so it must be relative and only consist of normal +/// `file_path` can be chosen by users, so it must be relative and only consist of normal /// components, otherwise the write fails. /// /// The Pod can write to the volume, so since the last publish it may have replaced any entry in @@ -411,62 +411,57 @@ async fn mark_provisioned(target_path: &Path) -> Result<(), PublishError> { /// cannot redirect the write. It only writes into regular files: a FIFO or device node in place /// of the file is an error. fn write_secret_file( - root: &Path, - relative_path: &Path, + target_path: &Path, + file_path: &Path, contents: &[u8], ) -> Result<(), PublishError> { - // Ensure that `relative_path` is indeed a relative path + // An absolute path would leave the volume: joining it to `target_path` replaces + // `target_path` entirely. ensure!( - !relative_path.has_root(), - publish_error::InvalidAbsolutePathSnafu { - path: relative_path - } + !file_path.has_root(), + publish_error::InvalidAbsolutePathSnafu { path: file_path } ); - // Ensure that all components of the path are "normal" - // Examples for not-normal are things like "." and ".." + // Only normal components, so that `..` cannot traverse out of the volume. ensure!( - relative_path + file_path .components() .all(|c| matches!(c, Component::Normal(_))), - publish_error::InvalidComponentsSnafu { - path: relative_path - } + publish_error::InvalidComponentsSnafu { path: file_path } ); - // This creates the directory structure needed to get to the relative directory - // making sure to not follow symlinks and so on - let directory_flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC; - let file_name = relative_path + let file_name = file_path .file_name() - .context(publish_error::InvalidComponentsSnafu { - path: relative_path, - })?; + .context(publish_error::InvalidComponentsSnafu { path: file_path })?; + + // RDONLY: a directory can only be opened read-only, and the descriptor is only used as the + // base for the next `openat`/`mkdirat`. + // DIRECTORY: fail if the entry is not a directory. + // NOFOLLOW: fail if the entry is a symlink instead of following it. + // CLOEXEC: do not leak the descriptor into child processes. + let directory_flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC; - let mut path = root.to_path_buf(); - let mut directory = openat(CWD, root, directory_flags, Mode::empty()) + let mut path = target_path.to_path_buf(); + let mut directory = openat(CWD, target_path, directory_flags, Mode::empty()) .map_err(std::io::Error::from) .context(publish_error::CreateDirSnafu { path: &path })?; - for component in relative_path - .parent() - .into_iter() - .flat_map(Path::components) - { - let name = component.as_os_str(); - path.push(name); - match mkdirat(&directory, name, Mode::from_raw_mode(0o777)) { + for component in file_path.parent().into_iter().flat_map(Path::components) { + let component = component.as_os_str(); + path.push(component); + // Directories need the execute bit to be entered, so this is 0o750 rather than the 0o640 + // of the files. + match mkdirat(&directory, component, Mode::from_raw_mode(0o750)) { Ok(()) | Err(Errno::EXIST) => {} Err(err) => { return Err(std::io::Error::from(err)) .context(publish_error::CreateDirSnafu { path }); } } - directory = openat(&directory, name, directory_flags, Mode::empty()) + directory = openat(&directory, component, directory_flags, Mode::empty()) .map_err(std::io::Error::from) .context(publish_error::CreateDirSnafu { path: &path })?; } - // And now finally we create the actual file from `contents` path.push(file_name); let file = openat( &directory, @@ -476,7 +471,10 @@ fn write_secret_file( // A repeated publish rewrites existing files, and regenerated keys and certificates // vary in length, so a shorter file would keep trailing bytes of the previous one. | OFlags::TRUNC + // Fail on a symlink instead of writing to its target. | OFlags::NOFOLLOW + // Opening a FIFO without a reader would otherwise block; the type check below then + // rejects it. | OFlags::NONBLOCK | OFlags::CLOEXEC, // User: root/secret-operator From 0f5d8d15f66ef638b336a7b94b631055f03c57e3 Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Fri, 13 Nov 2026 15:24:45 +0100 Subject: [PATCH 5/5] test: Pin secret file and directory modes, correct the NONBLOCK comment --- rust/operator-binary/src/csi_server/node.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/rust/operator-binary/src/csi_server/node.rs b/rust/operator-binary/src/csi_server/node.rs index ef29830c..5a2d4692 100644 --- a/rust/operator-binary/src/csi_server/node.rs +++ b/rust/operator-binary/src/csi_server/node.rs @@ -473,8 +473,8 @@ fn write_secret_file( | OFlags::TRUNC // Fail on a symlink instead of writing to its target. | OFlags::NOFOLLOW - // Opening a FIFO without a reader would otherwise block; the type check below then - // rejects it. + // Opening a FIFO would otherwise block until a reader appears. With it the open fails + // if there is no reader, and the type check below rejects a FIFO that has one. | OFlags::NONBLOCK | OFlags::CLOEXEC, // User: root/secret-operator @@ -635,7 +635,7 @@ impl Node for SecretProvisionerNode { #[cfg(test)] mod tests { - use std::path::Path; + use std::{os::unix::fs::MetadataExt as _, path::Path}; use rustix::fs::{CWD, FileType, Mode, mknodat}; @@ -715,6 +715,17 @@ mod tests { let contents = std::fs::read(volume.path().join("nested/tls.crt")).expect("secret file is readable"); assert_eq!(contents, b"certificate"); + + // The umask can only remove bits, so check for an upper bound rather than exact modes. + let directory_mode = std::fs::metadata(volume.path().join("nested")) + .expect("directory metadata is readable") + .mode(); + assert_ne!(directory_mode & 0o100, 0, "owner can enter the directory"); + assert_eq!(directory_mode & 0o777 & !0o750, 0); + let file_mode = std::fs::metadata(volume.path().join("nested/tls.crt")) + .expect("secret file metadata is readable") + .mode(); + assert_eq!(file_mode & 0o777 & !0o640, 0); } #[test]