fix(security): prevent path traversal in agent definition prompt file loading#1904
Merged
senamakel merged 7 commits intoMay 17, 2026
Conversation
… loading
Agent definition TOML files loaded from {workspace}/agents/ and
~/.openhuman/agents/ support [system_prompt] file = "..." which reads
prompt content from disk. The file path was joined with the workspace
directory without canonicalization, allowing path traversal (e.g.
file = "../../etc/shadow") to read arbitrary files.
Impact:
- Arbitrary file read from agent process context
- File content injected into LLM system prompt (data exfiltration
to cloud LLM provider)
- Attacker-controlled file content becomes indirect prompt injection
Fix: Add canonicalize() + workspace-bounding check before reading,
following the same pattern used in file_read.rs and file_write.rs.
Fixes: tinyhumansai#1898
Signed-off-by: Jason L <jason@outland.art>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSession builder and subagent tool-prep canonicalize candidate prompt file paths and enforce workspace-root containment before reading; escaping paths log warnings and return an empty prompt body instead of reading arbitrary files. ChangesPath traversal prevention in prompt file loading
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/agent/harness/subagent_runner/tool_prep.rs (1)
221-260:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't classify a missing override file as a traversal attempt.
validate_prompt_path()callscanonicalize(), which returns an error for non-existent paths. This causes the normal "no workspace override exists" case to fall into theescapes workspacewarning path, emitting false-positive security warnings that reduce the signal-to-noise ratio of real traversal detection.♻️ Proposed fix
let workspace_path = workspace_dir.join("agent").join("prompts").join(path); - if let Ok(resolved) = validate_prompt_path(&workspace_path, workspace_dir) { - if resolved.is_file() { - return std::fs::read_to_string(&resolved).map_err(|e| { - SubagentRunError::PromptLoad { - path: resolved.display().to_string(), - source: e, - } - }); - } - } else { - tracing::warn!( - "[subagent_runner] prompt path escapes workspace, skipping: {}", - workspace_path.display() - ); + if workspace_path.is_file() { + if let Ok(resolved) = validate_prompt_path(&workspace_path, workspace_dir) { + return std::fs::read_to_string(&resolved).map_err(|e| { + SubagentRunError::PromptLoad { + path: resolved.display().to_string(), + source: e, + } + }); + } + tracing::warn!( + "[subagent_runner] prompt path escapes workspace, skipping: {}", + workspace_path.display() + ); } @@ let workspace_root_path = workspace_dir.join(path); - if let Ok(resolved) = validate_prompt_path(&workspace_root_path, workspace_dir) { - if resolved.is_file() { - return std::fs::read_to_string(&resolved).map_err(|e| { - SubagentRunError::PromptLoad { - path: resolved.display().to_string(), - source: e, - } - }); - } - } else { - tracing::warn!( - "[subagent_runner] fallback prompt path escapes workspace, skipping: {}", - workspace_root_path.display() - ); + if workspace_root_path.is_file() { + if let Ok(resolved) = validate_prompt_path(&workspace_root_path, workspace_dir) { + return std::fs::read_to_string(&resolved).map_err(|e| { + SubagentRunError::PromptLoad { + path: resolved.display().to_string(), + source: e, + } + }); + } + tracing::warn!( + "[subagent_runner] fallback prompt path escapes workspace, skipping: {}", + workspace_root_path.display() + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/subagent_runner/tool_prep.rs` around lines 221 - 260, The false-positive warnings come from calling validate_prompt_path (which canonicalizes) on paths that don't exist; change the logic to first check existence and only call validate_prompt_path when the candidate file exists. Specifically, before calling validate_prompt_path(&workspace_path, workspace_dir) and before calling validate_prompt_path(&workspace_root_path, workspace_dir), add an exists() check on workspace_path and workspace_root_path respectively and if the file doesn't exist, treat it as a missing override (returning an empty body or continuing) rather than invoking validate_prompt_path and emitting the "escapes workspace" warning; this avoids touching validate_prompt_path and keeps SubagentRunError::PromptLoad behavior unchanged for actual read errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/openhuman/agent/harness/subagent_runner/tool_prep.rs`:
- Around line 221-260: The false-positive warnings come from calling
validate_prompt_path (which canonicalizes) on paths that don't exist; change the
logic to first check existence and only call validate_prompt_path when the
candidate file exists. Specifically, before calling
validate_prompt_path(&workspace_path, workspace_dir) and before calling
validate_prompt_path(&workspace_root_path, workspace_dir), add an exists() check
on workspace_path and workspace_root_path respectively and if the file doesn't
exist, treat it as a missing override (returning an empty body or continuing)
rather than invoking validate_prompt_path and emitting the "escapes workspace"
warning; this avoids touching validate_prompt_path and keeps
SubagentRunError::PromptLoad behavior unchanged for actual read errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b4d35f54-be2f-4dec-8ef8-5023e1bf45d3
📒 Files selected for processing (2)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/subagent_runner/tool_prep.rs
validate_prompt_path() used canonicalize() which returns Err for non-existent files, causing normal 'no workspace override' misses to emit false-positive security warnings. Now returns Ok(None) for missing files (silent skip) and Err only for actual traversal attempts. Normal miss log level downgraded from warn to debug. Addresses CodeRabbit review feedback on PR tinyhumansai#1904. Signed-off-by: Jason L <jason@outland.art>
…positives Adopt CodeRabbit's suggestion: check .is_file() before calling validate_prompt_path() so that normal missing-override cases skip canonicalization entirely. This keeps the validation function simple and eliminates false-positive 'escapes workspace' warnings for non-existent files. Revert validate_prompt_path to return Result<PathBuf, String> (no Option wrapper needed since callers pre-check existence). Addresses CodeRabbit review on PR tinyhumansai#1904. Signed-off-by: Jason L <jason@outland.art>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/openhuman/agent/harness/subagent_runner/tool_prep.rs (2)
185-203: ⚖️ Poor tradeoffConsider extracting
validate_prompt_pathto a shared module.This function is duplicated nearly verbatim in
session/builder.rs(lines 1337-1355). Both files now implement the same canonicalize +starts_withpattern. Given that this is a security-sensitive function, having a single source of truth would reduce the risk of divergence during future maintenance.A natural home could be alongside the existing
file_read.rs/file_write.rscanonicalization pattern mentioned in the PR objectives, or in a shared path utilities module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/subagent_runner/tool_prep.rs` around lines 185 - 203, The validate_prompt_path function is duplicated; extract its logic into a shared utility (e.g., a path_utils module with a public function validate_prompt_path or validate_path_within_root) and replace the local implementations in both places with calls to that single function; ensure the new function preserves the canonicalize + starts_with behavior and returns Result<PathBuf, String>, update imports/usages in the modules that previously had the duplicated validate_prompt_path, and add or move any relevant unit tests to cover the shared implementation.
180-203: 💤 Low valueDocumentation structure issue:
load_prompt_sourcedocstring will merge intovalidate_prompt_path.Lines 175-179 contain the original docstring for
load_prompt_source("Resolve a PromptSource to its raw markdown body..."), but this text now immediately precedes the newvalidate_prompt_pathdocstring without a blank line separator. The merged docstring will confuse readers since the first paragraph describes PromptSource resolution, not path validation.📝 Proposed fix: add blank line to separate docstrings
/// workspace `prompts/` directory or the agent crate's bundled prompts. /// + /// Validate that a prompt file path resolves within the workspace root. /// Prevents path traversal via `..` segments in agent definition TOML files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/subagent_runner/tool_prep.rs` around lines 180 - 203, The docstring for load_prompt_source is immediately followed by the doc comment for validate_prompt_path causing the two to merge; insert a blank line (an empty line) between the end of the load_prompt_source docstring and the start of the validate_prompt_path doc comment so the documentation for load_prompt_source and the validate_prompt_path function remain separate and render correctly (look for the doc comment above load_prompt_source and the /// comment block immediately before the fn validate_prompt_path).src/openhuman/agent/harness/session/builder.rs (1)
1332-1355: 💤 Low valueDocumentation structure issue: misplaced docstring will merge into
validate_prompt_path.The docstring at lines 1316-1331 (for
prefetch_tool_memory_rules_blocking) is immediately followed by this new function's docstring without a blank line separator. In Rust, consecutive///comments form a single doc block attached to the next item, sovalidate_prompt_pathwill inherit the unrelated "Best-effort synchronous prefetch of eager tool-scoped rules" documentation, andprefetch_tool_memory_rules_blockingwill lose its docstring entirely.📝 Proposed fix: add blank line to separate docstrings
/// merely seeds the rules that exist at session start. + /// Validate that a prompt file path resolves within the workspace root. /// Prevents path traversal via `..` segments in agent definition TOML files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/session/builder.rs` around lines 1332 - 1355, The documentation comments for prefetch_tool_memory_rules_blocking and validate_prompt_path are running together, causing the latter to inherit the former's docstring; insert a single blank line (an empty line) between the closing /// docblock for prefetch_tool_memory_rules_blocking and the /// docblock (or the fn) for validate_prompt_path so each function has its own doc comment; locate the doc comments immediately above prefetch_tool_memory_rules_blocking and validate_prompt_path in builder.rs and add the separation to restore correct Rust doc attachment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/agent/harness/session/builder.rs`:
- Around line 1332-1355: The documentation comments for
prefetch_tool_memory_rules_blocking and validate_prompt_path are running
together, causing the latter to inherit the former's docstring; insert a single
blank line (an empty line) between the closing /// docblock for
prefetch_tool_memory_rules_blocking and the /// docblock (or the fn) for
validate_prompt_path so each function has its own doc comment; locate the doc
comments immediately above prefetch_tool_memory_rules_blocking and
validate_prompt_path in builder.rs and add the separation to restore correct
Rust doc attachment.
In `@src/openhuman/agent/harness/subagent_runner/tool_prep.rs`:
- Around line 185-203: The validate_prompt_path function is duplicated; extract
its logic into a shared utility (e.g., a path_utils module with a public
function validate_prompt_path or validate_path_within_root) and replace the
local implementations in both places with calls to that single function; ensure
the new function preserves the canonicalize + starts_with behavior and returns
Result<PathBuf, String>, update imports/usages in the modules that previously
had the duplicated validate_prompt_path, and add or move any relevant unit tests
to cover the shared implementation.
- Around line 180-203: The docstring for load_prompt_source is immediately
followed by the doc comment for validate_prompt_path causing the two to merge;
insert a blank line (an empty line) between the end of the load_prompt_source
docstring and the start of the validate_prompt_path doc comment so the
documentation for load_prompt_source and the validate_prompt_path function
remain separate and render correctly (look for the doc comment above
load_prompt_source and the /// comment block immediately before the fn
validate_prompt_path).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 843a05eb-c7f0-4703-b856-d87cd0c192a0
📒 Files selected for processing (2)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/subagent_runner/tool_prep.rs
Remove duplicate validate_prompt_path implementations from session/builder.rs and subagent_runner/tool_prep.rs. Replace with a single shared function in security/policy.rs, alongside existing path validation helpers (is_path_allowed, is_resolved_path_allowed). Also fixes two docstring merge issues where validate_prompt_path docs were attached to the wrong function due to missing blank lines. Addresses CodeRabbit review feedback on PR tinyhumansai#1904. Signed-off-by: Jason L <jason@outland.art>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/agent/harness/session/builder.rs (1)
822-829:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate against the prompt directory, not the whole workspace.
path = "../../.env"still passes this check because the resolved path only has to stay underconfig.workspace_dir. That lets an agent definition pull arbitrary workspace files into the hidden system prompt, which is still a meaningful exfiltration/prompt-injection path. The root should be theagent/promptsdirectory you joined from.Suggested fix
- let workspace_path = config - .workspace_dir - .join("agent") - .join("prompts") - .join(path); + let prompt_root = config.workspace_dir.join("agent").join("prompts"); + let workspace_path = prompt_root.join(path); let body_text = if workspace_path.is_file() { - match crate::openhuman::security::validate_path_within_root(&workspace_path, &config.workspace_dir) { + match crate::openhuman::security::validate_path_within_root( + &workspace_path, + &prompt_root, + ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/session/builder.rs` around lines 822 - 829, The code currently validates workspace_path against config.workspace_dir which allows paths like "../../.env"; change the validation root to the prompts directory you constructed: create a prompts_root (e.g., let prompts_root = config.workspace_dir.join("agent").join("prompts")) and call crate::openhuman::security::validate_path_within_root(&workspace_path, &prompts_root) (instead of &config.workspace_dir) so workspace_path is required to be inside the agent/prompts directory; update any variable names (workspace_path, prompts_root) accordingly and preserve existing error handling and Ok(resolved) usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/openhuman/agent/harness/session/builder.rs`:
- Around line 822-829: The code currently validates workspace_path against
config.workspace_dir which allows paths like "../../.env"; change the validation
root to the prompts directory you constructed: create a prompts_root (e.g., let
prompts_root = config.workspace_dir.join("agent").join("prompts")) and call
crate::openhuman::security::validate_path_within_root(&workspace_path,
&prompts_root) (instead of &config.workspace_dir) so workspace_path is required
to be inside the agent/prompts directory; update any variable names
(workspace_path, prompts_root) accordingly and preserve existing error handling
and Ok(resolved) usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f36581e-63e0-4c68-accf-bfa223c95a62
📒 Files selected for processing (4)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/subagent_runner/tool_prep.rssrc/openhuman/security/mod.rssrc/openhuman/security/policy.rs
✅ Files skipped from review due to trivial changes (1)
- src/openhuman/security/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/agent/harness/subagent_runner/tool_prep.rs
Previously validated against the full workspace root, allowing path = "../../.env" to read any workspace file (e.g. .env with secrets) into the LLM system prompt. Now validates against the agent/prompts/ directory specifically, so prompt files can only come from that designated location. The workspace-wide fallback path in tool_prep.rs (for built-in archetype prompts like archetypes/researcher.md) still validates against the full workspace, which is correct for that use case. Addresses CodeRabbit review feedback on PR tinyhumansai#1904. Signed-off-by: Jason L <jason@outland.art>
senamakel
previously approved these changes
May 17, 2026
…_root - Add 5 unit tests for validate_path_within_root covering: contained path, parent-traversal (..), absolute escape, non-existent candidate, and symlink escape. Closes the coverage gap flagged during security audit (the function was added without its own tests). - Fix doc comment structure in tool_prep.rs: the blank non-/// line between the two doc paragraphs for load_prompt_source would split the comment block in rendered docs; replace with a /// continuation line. (addresses @coderabbitai on tool_prep.rs:180) - cargo fmt reorders pub use validate_path_within_root alphabetically in security/mod.rs (no semantic change).
senamakel
dismissed stale reviews from coderabbitai[bot] and themself
via
89c1301
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1898 — path traversal vulnerability in agent definition prompt file loading.
Problem
Agent definition TOML files from
{workspace}/agents/and~/.openhuman/agents/support[system_prompt] file = "..."which reads prompt content from disk. The file path was joined with the workspace directory without canonicalization, allowing../../etc/shadowstyle traversal.Impact:
Attack vector: A malicious workspace (e.g., cloned from untrusted source) containing
agents/malicious.tomlwithfile = "../../.ssh/id_rsa"Fix
Add
validate_prompt_path()that:.canonicalize()on the candidate path (resolves.., symlinks).canonicalize()on the workspace rootresolved.starts_with(&root)before readingThis follows the exact same pattern already used in
file_read.rsandfile_write.rs.Files Changed
src/openhuman/agent/harness/session/builder.rs— main session prompt loadingsrc/openhuman/agent/harness/subagent_runner/tool_prep.rs— sub-agent prompt loadingBoth paths now validate before reading. Path traversal attempts are logged as warnings and result in an empty prompt body (safe degradation).
Testing
The fix uses the same canonicalize + starts_with pattern that already has test coverage in
file_read.rs(including symlink escape tests). Manual verification:Security Audit Context
Discovered during independent security audit of OpenHuman v0.53.47. Full audit report covers 5 findings (3 medium, 2 low). This PR addresses the most impactful finding (M-01).
Summary by CodeRabbit