Consolidate Rust source security hardening - #632
Conversation
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (29)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR consolidates the remaining Rust-source security hardening workstreams (per the audit tracked in #610) into a single merge unit, focusing on preventing crashes, injection, secret leakage, and unbounded resource usage across the lexer, interpreter, transpiler, MCP/LSP server, and configuration/budget enforcement.
Changes:
- Hardened multiple untrusted-input and resource-exhaustion paths (numeric literals, file reads, outbound HTTP reads, subprocess capture, pattern compilation, MCP file resources).
- Improved security semantics and diagnostics (process allowlist-only direct-exec, DB URL redaction, cycle-safe value formatting/cloning, reduced MCP request/response logging).
- Added/updated comprehensive regression tests plus docs/changelog/dev-diary updates for the new/changed limits and behaviors.
Reviewed changes
Copilot reviewed 29 out of 31 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| wfl-lsp/src/mcp_server.rs | Confines MCP file/config reads to workspace-owned bounded .wfl sources; removes body echo logging; adds regressions. |
| tests/value_cycle_safety.rs | New Rust-level tests for cycle-safe formatting and deep cloning. |
| tests/transpiler_test.rs | Adds regressions ensuring untrusted strings can’t escape generated JS literals. |
| tests/subprocess_security_test.rs | Updates allowlist-only tests (Windows fixture) and adds chaining-block regression. |
| tests/http_outbound_budget_test.rs | New deterministic TCP-peer tests for outbound HTTP size/time/cancellation limits. |
| tests/execution_budget_test.rs | Adds config default/override assertions and an end-to-end file-read limit regression. |
| src/wfl_config/checker.rs | Adds max_file_read_size key validation and examples to config checker. |
| src/transpiler/javascript.rs | Centralizes JS string literal encoding and expands escaping for regex/text sinks; adds unit tests. |
| src/stdlib/filesystem.rs | Enforces file-read budget in count_lines and adds a budget regression test. |
| src/pattern/compiler.rs | Adds compile-time instruction ceiling + checked quantifier accounting; shares limits with lookbehind; adds tests. |
| src/parser/tests.rs | Adds parser regressions for descending/out-of-range pattern quantifiers. |
| src/parser/stmt/patterns.rs | Validates quantifier counts/ranges at parse time (checked conversions, descending-range rejection) + tests. |
| src/lexer/token.rs | Makes numeric literal lexing fallible and rejects non-finite floats (avoids panics/infinities). |
| src/lexer/tests.rs | Adds lexer regressions for oversized ints and overflowing float literals. |
| src/interpreter/value.rs | Makes Display/Debug cycle-aware + depth-bounded; memoizes deep clones to preserve cycles/aliases safely. |
| src/interpreter/tests.rs | Adds interpreter-level regression for displaying a self-referential list safely. |
| src/interpreter/mod.rs | Adds bounded file-read helper; streams/bounds outbound HTTP reads; bounds foreground subprocess capture and enforces deadlines/cancellation. |
| src/interpreter/database.rs | Redacts unsupported DB URL details to avoid credential leakage; adds regression. |
| src/interpreter/command_sanitizer.rs | Enforces allowlist-only as direct-exec only; binds allowlist path entries to canonical executable; adds regressions. |
| src/interpreter/bounded_buffer.rs | Fixes zero-sized buffer behavior and saturating accounting; adds regression. |
| src/exec/budget.rs | Adds max_file_read_bytes budget limit + typed breach; documents response limit applies outbound too; adds tests. |
| src/config.rs | Adds max_file_read_size config field, default, and loader support. |
| fuzz/Cargo.lock | Lockfile update for new dependency inclusion in fuzz workspace. |
| Docs/reference/configuration-reference.md | Documents allowlist-only semantics, response limits applying outbound, and new max_file_read_size. |
| Docs/04-advanced-features/subprocess-execution.md | Documents allowlist-only direct-exec boundary and bounded capture/timeout behavior. |
| Docs/04-advanced-features/interoperability.md | Documents outbound HTTP streaming/limits and timeout semantics inside/outside main loops. |
| Docs/04-advanced-features/file-io.md | Updates file I/O docs to reflect streaming enforcement + configurable max_file_read_size. |
| Dev diary/2026-07-16-cycle-safe-values.md | Dev diary entry explaining cycle-safe formatting and deep-clone memoization. |
| CHANGELOG.md | Adds Unreleased security notes for MCP confinement and DB URL redaction; notes cycle-safe values. |
| Cargo.toml | Enables reqwest streaming and adds encoding_rs dependency for bounded decoding. |
| Cargo.lock | Lockfile update for new dependency inclusion in main workspace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2715551fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| data: None, | ||
| }), | ||
| }, | ||
| let requested_path = match Url::parse(uri).ok().and_then(|url| url.to_file_path().ok()) { |
There was a problem hiding this comment.
When a workspace contains a .wfl file whose path has characters that must be percent-encoded (for example spaces or #), workspace://files still advertises the raw file:///... string, but this new Url::parse(uri) path rejects or mis-parses that URI before it reaches to_file_path(). An MCP client that reads the URI it just received from resources/list will get Invalid local file resource URI / not found for valid WFL files; either generate encoded file URLs in the listing or keep a decoder that accepts the server’s existing raw format.
Useful? React with 👍 / 👎.
| let path = entry.path(); | ||
| if path.extension().and_then(|s| s.to_str()) == Some("wfl") | ||
| && let Ok(content) = fs::read_to_string(&path) | ||
| && let Ok(content) = Self::read_bounded_text_file(&path) | ||
| { |
| let path = entry.path(); | ||
| if path.extension().and_then(|s| s.to_str()) == Some("wfl") | ||
| && let Ok(content) = fs::read_to_string(&path) | ||
| && let Ok(content) = Self::read_bounded_text_file(&path) | ||
| { | ||
| let diagnostics = self.core.analyze_document(&content); |
Summary
This consolidates the ten remaining Rust-source security hardening PRs into one review and merge unit, based on current
mainafter #630 and #631 merged.Included fixes:
Why
Each fix addresses an independent source-level security or resource-exhaustion finding from the audit tracked in #610. Combining them removes repeated merge-conflict churn while retaining the intended protections, tests, documentation, changelog entries, and required lockfile updates.
Integration notes
The combined tree preserves both sides of the only shared Rust conflict:
It also preserves the security changelog entries from #628 and #629 alongside the package hardening already merged through #630 and #631.
Validation
git diff --checkpassedSupersedes #620, #621, #622, #623, #624, #625, #626, #627, #628, and #629.