Secret file path validation - #3791
Open
jmthomas wants to merge 6 commits into
Open
Conversation
SECRET FILE paths are destinations COSMOS writes and reads back, so they are now required to resolve inside /tmp (override with OPENC3_SECRET_FILE_DIR). Paths are validated when a plugin is installed and again at the read and write sinks in Ruby and Python. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace the manual dirname walk and string slicing with Pathname#ascend and #relative_path_from. Also reject a broken symlink in the path, which the previous exist? check skipped over: opening such a path for writing creates the file the symlink points at. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Compare the expanded path against the base directory instead of resolving it on disk. Validation now gives the same answer in every container, since plugin install and the operator write run in different ones and see different filesystems. The operator resolves symlinks itself before writing a secret, which is where the filesystem being checked is the one being written to. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
jmthomas
requested review from
clayandgen and
mcosgriff
and
a lite review from Copilot
August 31, 2026 00:00
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3791 +/- ##
==========================================
+ Coverage 79.22% 79.27% +0.05%
==========================================
Files 894 894
Lines 67034 67112 +78
Branches 2553 2601 +48
==========================================
+ Hits 53105 53201 +96
+ Misses 13267 13247 -20
- Partials 662 664 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens SECRET FILE handling by restricting secret file destinations to a controlled base directory (default /tmp, configurable via OPENC3_SECRET_FILE_DIR), reducing the risk of reading/writing arbitrary host filesystem paths via plugin configuration.
Changes:
- Add lexical secret file path validation (
secret_file_dir+validate_file_path) in both Ruby and Python Secrets utilities, and apply it toFILEsecret reads. - Enforce
SECRET/BRIDGE_SECRETvalidation in models at config-parse time and add operator-side checks to prevent symlink-based escapes when writing secrets. - Add unit tests and update docs/config metadata to document the new constraints.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| openc3/spec/utilities/secrets_spec.rb | New Ruby unit coverage for secret file dir + path validation and FILE secret setup behavior |
| openc3/spec/operators/microservice_operator_spec.rb | New operator specs covering safe secret file writes + symlink-related rejection |
| openc3/spec/models/microservice_model_spec.rb | New specs validating SECRET parsing + rejecting out-of-base FILE paths |
| openc3/spec/models/interface_model_spec.rb | New specs validating SECRET/BRIDGE_SECRET parsing + rejecting out-of-base FILE paths |
| openc3/python/test/utilities/test_secrets.py | New Python unit coverage for secret file dir + path validation and FILE secret setup behavior |
| openc3/python/openc3/utilities/secrets.py | Implement secret_file_dir/validate_file_path and enforce validation on FILE reads |
| openc3/lib/openc3/utilities/secrets.rb | Implement secret_file_dir/validate_file_path (+ containment helper) and enforce validation on FILE reads |
| openc3/lib/openc3/operators/microservice_operator.rb | Validate/normalize FILE secret destinations and add realpath/symlink checks before writing |
| openc3/lib/openc3/models/microservice_model.rb | Validate SECRET definitions (type + FILE path restriction) during config handling |
| openc3/lib/openc3/models/interface_model.rb | Validate SECRET/BRIDGE_SECRET definitions (type + FILE path restriction) during config handling |
| openc3/data/config/microservice.yaml | Document FILE path restriction in keyword parameter descriptions |
| openc3/data/config/interface_modifiers.yaml | Document FILE path restriction for SECRET and BRIDGE_SECRET |
| docs.openc3.com/docs/development/microservices.md | Update docs example and explain FILE secret destination restrictions |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Store the uppercased secret type and the expanded FILE path back into the parsed parameters so downstream ENV/FILE comparisons and file writes see canonical values. In the operator, validate the deepest existing directory before mkdir_p so a symlinked component can no longer cause directories to be created outside the secret file dir, and log distinct errors for a symlinked file versus a path escaping the base. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
clayandgen
reviewed
Sep 2, 2026
clayandgen
left a comment
clayandgen
left a comment
Contributor
There was a problem hiding this comment.
Include OPENC3_SECRET_FILE_DIR in the .env / compose.yaml ?
|
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
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.
What changed
Validate that file paths passed to the SECRET keyword must be contained within
/tmp(by default)Why it changed
Allowing any file path would allow users to locate sensitive files in the filesystem
Testing strategy
Unit tests