🛡️ Sentinel: [Security Enhancement] Restrict model loading paths by EffortlessSteven · Pull Request #753 · EffortlessMetrics/BitNet-rs

EffortlessSteven · 2026-02-26T02:30:25Z

This PR enhances security by allowing administrators to restrict the directories from which models can be loaded. Previously, any file with a valid extension could be loaded if the path was absolute.

This change:

Adds allowed_model_directories to SecurityConfig.
Updates ConfigBuilder to support BITNET_ALLOWED_MODEL_DIRECTORIES.
Implements path validation using std::path::Path::starts_with in SecurityValidator.
Adds tests to ensure paths outside the allowlist are rejected.

Verification:

Run cargo test -p bitnet-server security::tests::test_model_path_restriction
Verify existing tests pass.

PR created automatically by Jules for task 13531758205369670074 started by @EffortlessSteven

- Adds `allowed_model_directories` to `SecurityConfig` in `bitnet-server`. - Updates `ConfigBuilder` to parse `BITNET_ALLOWED_MODEL_DIRECTORIES` env var. - Enforces path restriction in `SecurityValidator::validate_model_request`. - Adds unit tests to verify behavior. - Resolves potential security issue where arbitrary paths could be loaded.

google-labs-jules · 2026-02-26T02:30:28Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

gemini-code-assist · 2026-02-26T02:30:29Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

coderabbitai · 2026-02-26T02:30:33Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch sentinel/restrict-model-paths-13531758205369670074

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR adds an allowlist mechanism to the BitNet server’s security configuration to restrict which directories model files may be loaded from, and wires it through environment configuration and validation.

Changes:

Added allowed_model_directories to SecurityConfig and its default value.
Added BITNET_ALLOWED_MODEL_DIRECTORIES support in ConfigBuilder::from_env.
Enforced allowlist-based path validation in SecurityValidator::validate_model_request, with a new unit test.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`crates/bitnet-server/src/security.rs`	Adds `allowed_model_directories`, implements directory allowlist checks for model paths, and adds unit tests.
`crates/bitnet-server/src/config.rs`	Adds env var parsing for `BITNET_ALLOWED_MODEL_DIRECTORIES` into the security config.
`.jules/sentinel.md`	Documents the new “Unrestricted Model Loading Path” security learning and intended prevention.

Copilot · 2026-02-26T02:39:18Z

.jules/sentinel.md

+## 2025-06-03 - Unrestricted Model Loading Path
+**Vulnerability:** The server allowed loading model files from any path on the filesystem (e.g., via absolute paths) provided the file extension matched `.gguf` or `.safetensors`. This could allow attackers to probe for the existence of files or load sensitive data if it happened to have the correct extension.
+**Learning:** Checking for file extensions and blocking `..` is insufficient for path security when absolute paths are allowed. Always restrict file operations to a specific root directory or allowlist.
+**Prevention:** Implement a configuration option (`allowed_model_directories`) to restrict file loading to specific directories. Use `std::path::Path::starts_with` for robust path prefix checking, rather than string manipulation which can be bypassed (e.g., `/var/log` matching `/var/login`). Ensure existing path traversal protections are maintained.


This prevention note recommends Path::starts_with, but starts_with alone does not protect against symlink escapes (a symlink within an allowed directory pointing outside). Consider amending this entry to mention canonicalization / realpath checks (or symlink restrictions) as part of the prevention guidance.

Suggested change

**Prevention:** Implement a configuration option (`allowed_model_directories`) to restrict file loading to specific directories. Use `std::path::Path::starts_with` for robust path prefix checking, rather than string manipulation which can be bypassed (e.g., `/var/log` matching `/var/login`). Ensure existing path traversal protections are maintained.

**Prevention:** Implement a configuration option (`allowed_model_directories`) to restrict file loading to specific directories. Canonicalize both the candidate model path and each allowed directory (e.g., via `std::fs::canonicalize`) and then use `std::path::Path::starts_with` on these canonical paths, rather than doing string-based prefix checks. This avoids simple bypasses (e.g., `/var/log` vs `/var/login`) and reduces the risk of symlink escapes out of the allowed directories; ensure existing path traversal protections are maintained.

Copilot · 2026-02-26T02:39:19Z

crates/bitnet-server/src/security.rs

@@ -21,6 +21,7 @@ pub struct SecurityConfig {
    pub max_prompt_length: usize,
    pub max_tokens_per_request: u32,
    pub allowed_origins: Vec<String>,


SecurityConfig is Deserialized from TOML via toml::from_str, but the new non-optional allowed_model_directories field has no serde default. Any existing config file that omits this new key will fail to parse. Add #[serde(default)] to this field (or #[serde(default)] on the struct) so older configs remain loadable.

Suggested change

pub allowed_origins: Vec<String>,

pub allowed_origins: Vec<String>,

#[serde(default)]

Copilot · 2026-02-26T02:39:19Z

crates/bitnet-server/src/config.rs

+            self.config.security.allowed_model_directories =
+                dirs.split(',').map(|s| s.trim().to_string()).collect();


Parsing BITNET_ALLOWED_MODEL_DIRECTORIES should drop empty entries after trimming. As written, a value like "/models," (or just ",") produces an empty string in the allowlist, and Path::starts_with("") effectively makes every path allowed, defeating the restriction.

Suggested change

self.config.security.allowed_model_directories =

dirs.split(',').map(|s| s.trim().to_string()).collect();

self.config.security.allowed_model_directories = dirs

.split(',')

.map(|s| s.trim())

.filter(|s| !s.is_empty())

.map(|s| s.to_string())

.collect();

Copilot · 2026-02-26T02:39:19Z

crates/bitnet-server/src/security.rs

+        // Check allowed directories
+        if !self.config.allowed_model_directories.is_empty() {
+            let path = std::path::Path::new(model_path);
+            let mut allowed = false;
+            for dir in &self.config.allowed_model_directories {
+                if path.starts_with(dir) {
+                    allowed = true;
+                    break;
+                }
+            }
+
+            if !allowed {
+                return Err(ValidationError::InvalidFieldValue(
+                    "Model path not in allowed directories".to_string(),
+                ));
+            }
+        }


The allowlist check uses a purely lexical Path::starts_with against unvalidated strings. Two concrete problems: (1) an empty allowlist entry (""), whether from env or config, will match any path; (2) without canonicalizing/resolving symlinks, a path inside an allowed directory can still escape via a symlink that points outside. Consider rejecting empty dirs and performing a canonicalized comparison (e.g., canonicalize model path and allowed dirs) so the restriction reflects the real filesystem location.

Copilot · 2026-02-26T02:39:19Z

crates/bitnet-server/src/security.rs

+        assert!(matches!(
+            validator.validate_model_request("/models/../secret.gguf"),
+            Err(ValidationError::InvalidFieldValue(msg)) if msg == "Invalid characters in model path"
+        ));


The new tests don't cover the empty-entry edge case for allowed_model_directories (e.g., env var "/models," -> vec!["/models", ""]). Add an assertion that an empty entry does not cause unrestricted access (and ideally that it is ignored/validated as an error).

Suggested change

));

));

// Case 3: Config with an empty entry should not grant unrestricted access

let mut config = SecurityConfig::default();

config.allowed_model_directories = vec!["/models".to_string(), "".to_string()];

let validator = SecurityValidator::new(config);

match validator {

// If the validator is constructed successfully, the empty entry must not

// cause unrestricted model path access.

Ok(validator) => {

// Still allow paths under the valid prefix.

assert!(validator.validate_model_request("/models/llama.gguf").is_ok());

// But disallow paths outside the allowed directories.

assert!(matches!(

validator.validate_model_request("/tmp/model.gguf"),

Err(ValidationError::InvalidFieldValue(_))

));

}

// Alternatively, construction may fail if empty entries are rejected up front.

Err(_err) => {

// Treat rejecting the configuration as acceptable behavior.

// The important guarantee is that an empty entry never widens access.

}

}

Copilot AI review requested due to automatic review settings February 26, 2026 02:30

Copilot started reviewing on behalf of EffortlessSteven February 26, 2026 02:30 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🛡️ Sentinel: [Security Enhancement] Restrict model loading paths#753

🛡️ Sentinel: [Security Enhancement] Restrict model loading paths#753
EffortlessSteven wants to merge 1 commit intomainfrom
sentinel/restrict-model-paths-13531758205369670074

EffortlessSteven commented Feb 26, 2026

Uh oh!

google-labs-jules bot commented Feb 26, 2026

Uh oh!

gemini-code-assist bot commented Feb 26, 2026

Uh oh!

coderabbitai bot commented Feb 26, 2026

Review skipped

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	pub allowed_origins: Vec<String>,
	pub allowed_origins: Vec<String>,
	#[serde(default)]

		self.config.security.allowed_model_directories =
		dirs.split(',').map(\|s\| s.trim().to_string()).collect();

-        ));
+        ));
+        // Case 3: Config with an empty entry should not grant unrestricted access
+        let mut config = SecurityConfig::default();
+        config.allowed_model_directories = vec!["/models".to_string(), "".to_string()];
+        let validator = SecurityValidator::new(config);
+        match validator {
+            // If the validator is constructed successfully, the empty entry must not
+            // cause unrestricted model path access.
+            Ok(validator) => {
+                // Still allow paths under the valid prefix.
+                assert!(validator.validate_model_request("/models/llama.gguf").is_ok());
+                // But disallow paths outside the allowed directories.
+                assert!(matches!(
+                    validator.validate_model_request("/tmp/model.gguf"),
+                    Err(ValidationError::InvalidFieldValue(_))
+                ));
+            }
+            // Alternatively, construction may fail if empty entries are rejected up front.
+            Err(_err) => {
+                // Treat rejecting the configuration as acceptable behavior.
+                // The important guarantee is that an empty entry never widens access.
+            }
+        }

Conversation

EffortlessSteven commented Feb 26, 2026

Uh oh!

google-labs-jules bot commented Feb 26, 2026

Uh oh!

gemini-code-assist bot commented Feb 26, 2026

Uh oh!

coderabbitai bot commented Feb 26, 2026

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants